diff --git a/tools/cpplint.py b/tools/cpplint.py old mode 100644 new mode 100755 index d96713aa80..8afcae72a3 --- a/tools/cpplint.py +++ b/tools/cpplint.py @@ -44,7 +44,9 @@ same line, but it is far from perfect (in either direction). import codecs import copy import getopt +import glob import logging +import itertools import math # for log import os import re @@ -52,24 +54,53 @@ import sre_compile import string import sys import unicodedata +import xml.etree.ElementTree try: xrange except NameError: xrange = range +# if empty, use defaults +_header_extensions = set([]) + +# if empty, use defaults +_valid_extensions = set([]) +# Files with any of these extensions are considered to be +# header files (and will undergo different style checks). +# This set can be extended by using the --headers +# option (also supported in CPPLINT.cfg) +def GetHeaderExtensions(): + if not _header_extensions: + return set(['h', 'hpp', 'hxx', 'h++', 'cuh']) + return _header_extensions + +# The allowed extensions for file names +# This is set by --extensions flag +def GetAllExtensions(): + if not _valid_extensions: + return GetHeaderExtensions().union(set(['c', 'cc', 'cpp', 'cxx', 'c++', 'cu'])) + return _valid_extensions + +def GetNonHeaderExtensions(): + return GetAllExtensions().difference(GetHeaderExtensions()) logger = logging.getLogger('testrunner') _USAGE = """ -Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] - [--counting=total|toplevel|detailed] [--root=subdir] - [--linelength=digits] [--logfile=filename] +Syntax: cpplint.py [--verbose=#] [--output=emacs|eclipse|vs7|junit] + [--filter=-x,+y,...] + [--counting=total|toplevel|detailed] [--repository=path] + [--root=subdir] [--linelength=digits] [--recursive] + [--exclude=path] + [--headers=ext1,ext2] + [--logfile=filename] + [--extensions=hpp,cpp,...] [file] ... The style guidelines this tries to follow are those in - https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml + https://google.github.io/styleguide/cppguide.html Every problem is given a confidence score from 1-5, with 5 meaning we are certain of the problem, and 1 meaning it could be a legitimate construct. @@ -80,17 +111,26 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] suppresses errors of all categories on that line. The files passed in will be linted; at least one file must be provided. - Default linted extensions are .cc, .cpp, .cu, .cuh and .h. Change the - extensions with the --extensions flag. + Default linted extensions are %s. + Other file types will be ignored. + Change the extensions with the --extensions flag. Flags: - output=vs7 - By default, the output is formatted to ease emacs parsing. Visual Studio - compatible output (vs7) may also be used. Other formats are unsupported. + output=emacs|eclipse|vs7|junit + By default, the output is formatted to ease emacs parsing. Output + compatible with eclipse (eclipse), Visual Studio (vs7), and JUnit + XML parsers such as those used in Jenkins and Bamboo may also be + used. Other formats are unsupported. verbose=# Specify a number 0-5 to restrict errors to certain verbosity levels. + Errors with lower verbosity levels have lower confidence and are more + likely to be false positives. + + quiet + Supress output other than linting errors, such as information about + which files have been processed and excluded. filter=-x,+y,... Specify a comma-separated list of category-filters to apply: only @@ -114,17 +154,40 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] also be printed. If 'detailed' is provided, then a count is provided for each category like 'build/class'. - root=subdir - The root directory used for deriving header guard CPP variable. - By default, the header guard CPP variable is calculated as the relative - path to the directory that contains .git, .hg, or .svn. When this flag - is specified, the relative path is calculated from the specified - directory. If the specified directory does not exist, this flag is - ignored. + repository=path + The top level directory of the repository, used to derive the header + guard CPP variable. By default, this is determined by searching for a + path that contains .git, .hg, or .svn. When this flag is specified, the + given path is used instead. This option allows the header guard CPP + variable to remain consistent even if members of a team have different + repository root directories (such as when checking out a subdirectory + with SVN). In addition, users of non-mainstream version control systems + can use this flag to ensure readable header guard CPP variables. Examples: - Assuming that src/.git exists, the header guard CPP variables for - src/chrome/browser/ui/browser.h are: + Assuming that Alice checks out ProjectName and Bob checks out + ProjectName/trunk and trunk contains src/chrome/ui/browser.h, then + with no --repository flag, the header guard CPP variable will be: + + Alice => TRUNK_SRC_CHROME_BROWSER_UI_BROWSER_H_ + Bob => SRC_CHROME_BROWSER_UI_BROWSER_H_ + + If Alice uses the --repository=trunk flag and Bob omits the flag or + uses --repository=. then the header guard CPP variable will be: + + Alice => SRC_CHROME_BROWSER_UI_BROWSER_H_ + Bob => SRC_CHROME_BROWSER_UI_BROWSER_H_ + + root=subdir + The root directory used for deriving header guard CPP variables. This + directory is relative to the top level directory of the repository which + by default is determined by searching for a directory that contains .git, + .hg, or .svn but can also be controlled with the --repository flag. If + the specified directory does not exist, this flag is ignored. + + Examples: + Assuming that src is the top level directory of the repository, the + header guard CPP variables for src/chrome/browser/ui/browser.h are: No flag => CHROME_BROWSER_UI_BROWSER_H_ --root=chrome => BROWSER_UI_BROWSER_H_ @@ -137,14 +200,36 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] Examples: --linelength=120 + recursive + Search for files to lint recursively. Each directory given in the list + of files to be linted is replaced by all files that descend from that + directory. Files with extensions not in the valid extensions list are + excluded. + + exclude=path + Exclude the given path from the list of files to be linted. Relative + paths are evaluated relative to the current directory and shell globbing + is performed. This flag can be provided multiple times to exclude + multiple files. + + Examples: + --exclude=one.cc + --exclude=src/*.cc + --exclude=src/*.cc --exclude=test/*.cc + extensions=extension,extension,... The allowed file extensions that cpplint will check Examples: - --extensions=hpp,cpp + --extensions=%s - logfile=filename - Write TAP output to a logfile. + headers=extension,extension,... + The allowed header extensions that cpplint will consider to be header files + (by default, only files with extensions %s + will be assumed to be headers) + + Examples: + --headers=%s cpplint.py supports per-directory configurations specified in CPPLINT.cfg files. CPPLINT.cfg file can contain a number of key=value pairs. @@ -154,6 +239,7 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] filter=+filter1,-filter2,... exclude_files=regex linelength=80 + root=subdir "set noparent" option prevents cpplint from traversing directory tree upwards looking for more .cfg files in parent directories. This option @@ -165,22 +251,28 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] "exclude_files" allows to specify a regular expression to be matched against a file name. If the expression matches, the file is skipped and not run - through liner. + through the linter. - "linelength" allows to specify the allowed line length for the project. + "linelength" specifies the allowed line length for the project. + + The "root" option is similar in function to the --root flag (see example + above). CPPLINT.cfg has an effect on files in the same directory and all - sub-directories, unless overridden by a nested configuration file. + subdirectories, unless overridden by a nested configuration file. Example file: filter=-build/include_order,+build/include_alpha - exclude_files=.*\.cc + exclude_files=.*\\.cc The above example disables build/include_order warning and enables build/include_alpha as well as excludes all .cc from being processed by linter, in the current directory (where the .cfg - file is located) and all sub-directories. -""" + file is located) and all subdirectories. +""" % (list(GetAllExtensions()), + ','.join(list(GetAllExtensions())), + GetHeaderExtensions(), + ','.join(GetHeaderExtensions())) # We categorize each error message we print. Here are the categories. # We want an explicit list so we can list them all in cpplint --filter=. @@ -200,6 +292,7 @@ _ERROR_CATEGORIES = [ 'build/include_alpha', 'build/include_order', 'build/include_what_you_use', + 'build/namespaces_literals', 'build/namespaces', 'build/printf_format', 'build/storage_class', @@ -270,6 +363,7 @@ _DEFAULT_FILTERS = [ '-build/include', '-build/include_alpha', '-build/include_order', + '-build/include_subdir', '-legal/copyright', ] @@ -447,7 +541,8 @@ _THIRD_PARTY_HEADERS_PATTERN = re.compile( r'^(?:[^/]*[A-Z][^/]*\.h|lua\.h|lauxlib\.h|lualib\.h)$') # Pattern for matching FileInfo.BaseName() against test file name -_TEST_FILE_SUFFIX = r'(_test|_unittest|_regtest)$' +_test_suffixes = ['_test', '_regtest', '_unittest'] +_TEST_FILE_SUFFIX = '(' + '|'.join(_test_suffixes) + r')$' # Pattern that matches only complete whitespace, possibly across multiple lines. _EMPTY_CONDITIONAL_BODY_PATTERN = re.compile(r'^\s*$', re.DOTALL) @@ -461,7 +556,7 @@ _CHECK_MACROS = [ ] # Replacement macros for CHECK/DCHECK/EXPECT_TRUE/EXPECT_FALSE -_CHECK_REPLACEMENT = dict([(m, {}) for m in _CHECK_MACROS]) +_CHECK_REPLACEMENT = dict([(macro_var, {}) for macro_var in _CHECK_MACROS]) for op, replacement in [('==', 'EQ'), ('!=', 'NE'), ('>=', 'GE'), ('>', 'GT'), @@ -504,6 +599,7 @@ _ALT_TOKEN_REPLACEMENT = { _ALT_TOKEN_REPLACEMENT_PATTERN = re.compile( r'[ =()](' + ('|'.join(_ALT_TOKEN_REPLACEMENT.keys())) + r')(?=[ (]|$)') + # These constants define types of headers for use with # _IncludeState.CheckNextIncludeOrder(). _C_SYS_HEADER = 1 @@ -546,13 +642,54 @@ _error_suppressions = {} # This is set by --root flag. _root = None +# The top level repository directory. If set, _root is calculated relative to +# this directory instead of the directory containing version control artifacts. +# This is set by the --repository flag. +_repository = None + +# Files to exclude from linting. This is set by the --exclude flag. +_excludes = None + +# Whether to supress PrintInfo messages +_quiet = False + # The allowed line length of files. # This is set by --linelength flag. _line_length = 80 -# The allowed extensions for file names -# This is set by --extensions flag. -_valid_extensions = set(['cc', 'h', 'cpp', 'cu', 'cuh']) +try: + xrange(1, 0) +except NameError: + # -- pylint: disable=redefined-builtin + xrange = range + +try: + unicode +except NameError: + # -- pylint: disable=redefined-builtin + basestring = unicode = str + +try: + long(2) +except NameError: + # -- pylint: disable=redefined-builtin + long = int + +if sys.version_info < (3,): + # -- pylint: disable=no-member + # BINARY_TYPE = str + itervalues = dict.itervalues + iteritems = dict.iteritems +else: + # BINARY_TYPE = bytes + itervalues = dict.values + iteritems = dict.items + +def unicode_escape_decode(x): + if sys.version_info < (3,): + return codecs.unicode_escape_decode(x)[0] + else: + return x # {str, bool}: a map from error categories to booleans which indicate if the # category should be suppressed for every line. @@ -670,7 +807,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 GetNonHeaderExtensions() class _IncludeState(object): @@ -710,6 +847,8 @@ class _IncludeState(object): def __init__(self): self.include_list = [[]] + self._section = None + self._last_header = None self.ResetSection('') def FindHeader(self, header): @@ -853,9 +992,16 @@ class _CppLintState(object): # output format: # "emacs" - format that emacs can parse (default) + # "eclipse" - format that eclipse can parse # "vs7" - format that Microsoft Visual Studio 7 can parse + # "junit" - format that Jenkins, Bamboo, etc can parse self.output_format = 'emacs' + # For JUnit output, save errors and failures until the end so that they + # can be written into the XML + self._junit_errors = [] + self._junit_failures = [] + def SetOutputFormat(self, output_format): """Sets the output format for errors.""" self.output_format = output_format @@ -924,10 +1070,69 @@ class _CppLintState(object): def PrintErrorCounts(self): """Print a summary of errors by category, and the total.""" - for category, count in self.errors_by_category.iteritems(): - sys.stderr.write('Category \'%s\' errors found: %d\n' % + for category, count in sorted(iteritems(self.errors_by_category)): + self.PrintInfo('Category \'%s\' errors found: %d\n' % (category, count)) - sys.stderr.write('Total errors found: %d\n' % self.error_count) + if self.error_count > 0: + self.PrintInfo('Total errors found: %d\n' % self.error_count) + + def PrintInfo(self, message): + if not _quiet and self.output_format != 'junit': + sys.stderr.write(message) + + def PrintError(self, message): + if self.output_format == 'junit': + self._junit_errors.append(message) + else: + sys.stderr.write(message) + + def AddJUnitFailure(self, filename, linenum, message, category, confidence): + self._junit_failures.append((filename, linenum, message, category, + confidence)) + + def FormatJUnitXML(self): + num_errors = len(self._junit_errors) + num_failures = len(self._junit_failures) + + testsuite = xml.etree.ElementTree.Element('testsuite') + testsuite.attrib['name'] = 'cpplint' + testsuite.attrib['errors'] = str(num_errors) + testsuite.attrib['failures'] = str(num_failures) + + if num_errors == 0 and num_failures == 0: + testsuite.attrib['tests'] = str(1) + xml.etree.ElementTree.SubElement(testsuite, 'testcase', name='passed') + + else: + testsuite.attrib['tests'] = str(num_errors + num_failures) + if num_errors > 0: + testcase = xml.etree.ElementTree.SubElement(testsuite, 'testcase') + testcase.attrib['name'] = 'errors' + error = xml.etree.ElementTree.SubElement(testcase, 'error') + error.text = '\n'.join(self._junit_errors) + if num_failures > 0: + # Group failures by file + failed_file_order = [] + failures_by_file = {} + for failure in self._junit_failures: + failed_file = failure[0] + if failed_file not in failed_file_order: + failed_file_order.append(failed_file) + failures_by_file[failed_file] = [] + failures_by_file[failed_file].append(failure) + # Create a testcase for each file + for failed_file in failed_file_order: + failures = failures_by_file[failed_file] + testcase = xml.etree.ElementTree.SubElement(testsuite, 'testcase') + testcase.attrib['name'] = failed_file + failure = xml.etree.ElementTree.SubElement(testcase, 'failure') + template = '{0}: {1} [{2}] [{3}]' + texts = [template.format(f[1], f[2], f[3], f[4]) for f in failures] + failure.text = '\n'.join(texts) + + xml_decl = '\n' + return xml_decl + xml.etree.ElementTree.tostring(testsuite, 'utf-8').decode('utf-8') + _cpplint_state = _CppLintState() @@ -1173,7 +1378,7 @@ def Error(filename, linenum, category, confidence, message): if _ShouldPrintError(category, confidence, linenum): _cpplint_state.IncrementErrorCount(category) if _cpplint_state.output_format == 'vs7': - sys.stderr.write('%s(%s): %s [%s] [%d]\n' % ( + _cpplint_state.PrintError('%s(%s): warning: %s [%s] [%d]\n' % ( filename, linenum, message, category, confidence)) elif _cpplint_state.output_format == 'eclipse': sys.stderr.write('%s:%s: warning: %s [%s] [%d]\n' % ( @@ -1188,9 +1393,9 @@ def Error(filename, linenum, category, confidence, message): ' ...') logger.info(template % locals()) else: - sys.stderr.write('%s:%s: %s [%s] [%d]\n' % ( - filename, linenum, message, category, confidence)) - + final_message = '%s:%s: %s [%s] [%d]\n' % ( + filename, linenum, message, category, confidence) + sys.stderr.write(final_message) # Matches standard C++ escape sequences per 2.13.2.3 of the C++ standard. _RE_PATTERN_CLEANSE_LINE_ESCAPES = re.compile( @@ -1748,7 +1953,12 @@ def GetHeaderGuardCPPVariable(filename): fileinfo = FileInfo(filename) file_path_from_root = fileinfo.RepositoryName() if _root: - file_path_from_root = re.sub('^' + _root + os.sep, '', file_path_from_root) + suffix = os.sep + # On Windows using directory separator will leave us with + # "bogus escape error" unless we properly escape regex. + if suffix == '\\': + suffix += '\\' + file_path_from_root = re.sub('^' + _root + suffix, '', file_path_from_root) return re.sub(r'[^a-zA-Z0-9]', '_', file_path_from_root).upper() + '_' @@ -1775,6 +1985,11 @@ def CheckForHeaderGuard(filename, clean_lines, error): if Search(r'//\s*NOLINT\(build/header_guard\)', i): return + # Allow pragma once instead of header guards + for i in raw_lines: + if Search(r'^\s*#pragma\s+once', i): + return + cppvar = GetHeaderGuardCPPVariable(filename) ifndef = '' @@ -1851,28 +2066,30 @@ def CheckForHeaderGuard(filename, clean_lines, error): def CheckHeaderFileIncluded(filename, include_state, error): - """Logs an error if a .cc file does not include its header.""" + """Logs an error if a source file does not include its header.""" # Do not check test files fileinfo = FileInfo(filename) if Search(_TEST_FILE_SUFFIX, fileinfo.BaseName()): return - headerfile = filename[0:len(filename) - len(fileinfo.Extension())] + '.h' - if not os.path.exists(headerfile): - return - headername = FileInfo(headerfile).RepositoryName() - first_include = 0 - for section_list in include_state.include_list: - for f in section_list: - if headername in f[0] or f[0] in headername: - return - if not first_include: - first_include = f[1] + for ext in GetHeaderExtensions(): + basefilename = filename[0:len(filename) - len(fileinfo.Extension())] + headerfile = basefilename + '.' + ext + if not os.path.exists(headerfile): + continue + headername = FileInfo(headerfile).RepositoryName() + first_include = None + for section_list in include_state.include_list: + for f in section_list: + if headername in f[0] or f[0] in headername: + return + if not first_include: + first_include = f[1] - error(filename, first_include, 'build/include', 5, - '%s should include its header file %s' % (fileinfo.RepositoryName(), - headername)) + error(filename, first_include, 'build/include', 5, + '%s should include its header file %s' % (fileinfo.RepositoryName(), + headername)) def CheckForBadCharacters(filename, lines, error): @@ -1893,7 +2110,7 @@ def CheckForBadCharacters(filename, lines, error): error: The function to call with any errors found. """ for linenum, line in enumerate(lines): - if u'\ufffd' in line: + if unicode_escape_decode('\ufffd') in line: error(filename, linenum, 'readability/utf8', 5, 'Line contains invalid UTF-8 (or Unicode replacement character).') if '\0' in line: @@ -2537,7 +2754,7 @@ class NestingState(object): # class LOCKABLE API Object { # }; class_decl_match = Match( - r'^(\s*(?:template\s*<[\w\s<>,:]*>\s*)?' + r'^(\s*(?:template\s*<[\w\s<>,:=]*>\s*)?' r'(class|struct)\s+(?:[A-Z_]+\s+)*(\w+(?:::\w+)*))' r'(.*)$', line) if (class_decl_match and @@ -2785,6 +3002,7 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, constructor_args[i] = constructor_arg i += 1 + variadic_args = [arg for arg in constructor_args if '&&...' in arg] defaulted_args = [arg for arg in constructor_args if '=' in arg] noarg_constructor = (not constructor_args or # empty arg list # 'void' arg specifier @@ -2795,7 +3013,10 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, # all but at most one arg defaulted (len(constructor_args) >= 1 and not noarg_constructor and - len(defaulted_args) >= len(constructor_args) - 1)) + len(defaulted_args) >= len(constructor_args) - 1) or + # variadic arguments with zero or one argument + (len(constructor_args) <= 2 and + len(variadic_args) >= 1)) initializer_list_constructor = bool( onearg_constructor and Search(r'\bstd\s*::\s*initializer_list\b', constructor_args[0])) @@ -2808,7 +3029,7 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, onearg_constructor and not initializer_list_constructor and not copy_constructor): - if defaulted_args: + if defaulted_args or variadic_args: error(filename, linenum, 'runtime/explicit', 5, 'Constructors callable with one argument ' 'should be marked explicit.') @@ -3046,7 +3267,7 @@ def CheckComment(line, filename, linenum, next_line_start, error): # If the comment contains an alphanumeric character, there # should be a space somewhere between it and the // unless # it's a /// or //! Doxygen comment. - if (Match(r'(?://[^ ]*\w)|(?:////\s*$)', comment) and + if (Match(r'//[^ ]*\w', comment) and not Match(r'(///|//\!)(\s+|$)', comment)): error(filename, linenum, 'whitespace/comments', 4, 'Should have a space between // and comment') @@ -3568,7 +3789,6 @@ def IsDecltype(clean_lines, linenum, column): return True return False - def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error): """Checks for additional blank line issues related to sections. @@ -3901,6 +4121,14 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): # outputting warnings for the matching closing brace, if there are # nested blocks with trailing semicolons, we will get the error # messages in reversed order. + + # We need to check the line forward for NOLINT + raw_lines = clean_lines.raw_lines + ParseNolintSuppressions(filename, raw_lines[endlinenum-1], endlinenum-1, + error) + ParseNolintSuppressions(filename, raw_lines[endlinenum], endlinenum, + error) + error(filename, endlinenum, 'readability/braces', 4, "You don't need a ; after a }") @@ -3978,12 +4206,12 @@ def CheckEmptyBlockBody(filename, clean_lines, linenum, error): return if closing_linenum > opening_linenum: # Opening line after the {. Ignore comments here since we checked above. - body = list(opening_line[opening_pos+1:]) + bodylist = list(opening_line[opening_pos+1:]) # All lines until closing line, excluding closing line, with comments. - body.extend(clean_lines.raw_lines[opening_linenum+1:closing_linenum]) + bodylist.extend(clean_lines.raw_lines[opening_linenum+1:closing_linenum]) # Closing line before the }. Won't (and can't) have comments. - body.append(clean_lines.elided[closing_linenum][:closing_pos-1]) - body = '\n'.join(body) + bodylist.append(clean_lines.elided[closing_linenum][:closing_pos-1]) + body = '\n'.join(bodylist) else: # If statement has brackets and fits on a single line. body = opening_line[opening_pos+1:closing_pos-1] @@ -4318,7 +4546,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # Check if the line is a header guard. is_header_guard = False - if file_extension == 'h': + if file_extension in GetHeaderExtensions(): cppvar = GetHeaderGuardCPPVariable(filename) if (line.startswith('#ifndef %s' % cppvar) or line.startswith('#define %s' % cppvar) or @@ -4332,16 +4560,23 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # # The "$Id:...$" comment may also get very long without it being the # developers fault. + # + # Doxygen documentation copying can get pretty long when using an overloaded + # function declaration if (not line.startswith('#include') and not is_header_guard and not Match(r'^\s*//.*http(s?)://\S*$', line) and not Match(r'^\s*//\s*[^\s]*$', line) and - not Match(r'^// \$Id:.*#[0-9]+ \$$', line)): + not Match(r'^// \$Id:.*#[0-9]+ \$$', line) and + not Match(r'^\s*/// [@\\](copydoc|copydetails|copybrief) .*$', line)): line_width = GetLineWidth(line) if line_width > _line_length: error(filename, linenum, 'whitespace/line_length', 2, 'Lines should be <= %i characters long' % _line_length) if (cleansed_line.count(';') > 1 and + # allow simple single line lambdas + not Match(r'^[^{};]*\[[^\[\]]*\][^{}]*\{[^{}\n\r]*\}', + line) and # for loops are allowed two ;'s (and may run over two lines). cleansed_line.find('for') == -1 and (GetPreviousNonBlankLine(clean_lines, linenum)[0].find('for') == -1 or @@ -4401,8 +4636,11 @@ def _DropCommonSuffixes(filename): Returns: The filename with the common suffix removed. """ - for suffix in ('test.cc', 'regtest.cc', 'unittest.cc', - 'inl.h', 'impl.h', 'internal.h'): + for suffix in itertools.chain( + ('%s.%s' % (test_suffix.lstrip('_'), ext) + for test_suffix, ext in itertools.product(_test_suffixes, GetNonHeaderExtensions())), + ('%s.%s' % (suffix, ext) + for suffix, ext in itertools.product(['inl', 'imp', 'internal'], GetHeaderExtensions()))): if (filename.endswith(suffix) and len(filename) > len(suffix) and filename[-len(suffix) - 1] in ('-', '_')): return filename[:-len(suffix) - 1] @@ -4437,6 +4675,10 @@ def _ClassifyInclude(fileinfo, include, is_system): # those already checked for above. is_cpp_h = include in _CPP_HEADERS + # Headers with C++ extensions shouldn't be considered C system headers + if is_system and os.path.splitext(include)[1] in ['.hpp', '.hxx', '.h++']: + is_system = False + if is_system: if is_cpp_h: return _CPP_SYS_HEADER @@ -4449,9 +4691,11 @@ def _ClassifyInclude(fileinfo, include, is_system): target_dir, target_base = ( os.path.split(_DropCommonSuffixes(fileinfo.RepositoryName()))) include_dir, include_base = os.path.split(_DropCommonSuffixes(include)) + target_dir_pub = os.path.normpath(target_dir + '/../public') + target_dir_pub = target_dir_pub.replace('\\', '/') if target_base == include_base and ( include_dir == target_dir or - include_dir == os.path.normpath(target_dir + '/../public')): + include_dir == target_dir_pub): return _LIKELY_MY_HEADER # If the target and include share some initial basename @@ -4495,7 +4739,7 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): # naming convention but not the include convention. match = Match(r'#include\s*"([^/]+\.h)"', line) if match and not _THIRD_PARTY_HEADERS_PATTERN.match(match.group(1)): - error(filename, linenum, 'build/include', 4, + error(filename, linenum, 'build/include_subdir', 4, 'Include the directory when naming .h files') # we shouldn't include a file more than once. actually, there are a @@ -4510,11 +4754,16 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): error(filename, linenum, 'build/include', 4, '"%s" already included at %s:%s' % (include, filename, duplicate_line)) - elif (include.endswith('.cc') and + return + + for extension in GetNonHeaderExtensions(): + if (include.endswith('.' + extension) and os.path.dirname(fileinfo.RepositoryName()) != os.path.dirname(include)): - error(filename, linenum, 'build/include', 4, - 'Do not include .cc files from other packages') - elif not _THIRD_PARTY_HEADERS_PATTERN.match(include): + error(filename, linenum, 'build/include', 4, + 'Do not include .' + extension + ' files from other packages') + return + + if not _THIRD_PARTY_HEADERS_PATTERN.match(include): include_state.include_list[-1].append((include, linenum)) # We want to ensure that headers appear in the right order: @@ -4568,7 +4817,7 @@ def _GetTextInside(text, start_pattern): # Give opening punctuations to get the matching close-punctuations. matching_punctuation = {'(': ')', '{': '}', '[': ']'} - closing_punctuation = set(matching_punctuation.itervalues()) + closing_punctuation = set(itervalues(matching_punctuation)) # Find the position to start extracting text. match = re.search(start_pattern, text, re.M) @@ -4670,7 +4919,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, CheckGlobalStatic(filename, clean_lines, linenum, error) CheckPrintf(filename, clean_lines, linenum, error) - if file_extension == 'h': + if file_extension in GetHeaderExtensions(): # TODO(unknown): check that 1-arg constructors are explicit. # How to tell it's a constructor? # (handled in CheckForNonStandardConstructs for now) @@ -4731,9 +4980,14 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, % (match.group(1), match.group(2))) if Search(r'\busing namespace\b', line): - error(filename, linenum, 'build/namespaces', 5, - 'Do not use namespace using-directives. ' - 'Use using-declarations instead.') + if Search(r'\bliterals\b', line): + error(filename, linenum, 'build/namespaces_literals', 5, + 'Do not use namespace using-directives. ' + 'Use using-declarations instead.') + else: + error(filename, linenum, 'build/namespaces', 5, + 'Do not use namespace using-directives. ' + 'Use using-declarations instead.') # Detect variable-length arrays. match = Match(r'\s*(.+::)?(\w+) [a-z]\w*\[(.+)];', line) @@ -4777,7 +5031,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, # Check for use of unnamed namespaces in header files. Registration # macros are typically OK, so we allow use of "namespace {" on lines # that end with backslashes. - if (file_extension == 'h' + if (file_extension in GetHeaderExtensions() and Search(r'\bnamespace\s*{', line) and line[-1] != '\\'): error(filename, linenum, 'build/namespaces', 4, @@ -5377,7 +5631,7 @@ def FilesBelongToSameModule(filename_cc, filename_h): some false positives. This should be sufficiently rare in practice. Args: - filename_cc: is the path for the .cc file + filename_cc: is the path for the source (e.g. .cc) file filename_h: is the path for the header path Returns: @@ -5385,20 +5639,23 @@ def FilesBelongToSameModule(filename_cc, filename_h): bool: True if filename_cc and filename_h belong to the same module. string: the additional prefix needed to open the header file. """ - - fileinfo = FileInfo(filename_cc) - if not fileinfo.IsSource(): + fileinfo_cc = FileInfo(filename_cc) + if not fileinfo_cc.Extension().lstrip('.') in GetNonHeaderExtensions(): return (False, '') - filename_cc = filename_cc[:-len(fileinfo.Extension())] - matched_test_suffix = Search(_TEST_FILE_SUFFIX, fileinfo.BaseName()) + + fileinfo_h = FileInfo(filename_h) + if not fileinfo_h.Extension().lstrip('.') in GetHeaderExtensions(): + return (False, '') + + filename_cc = filename_cc[:-(len(fileinfo_cc.Extension()))] + matched_test_suffix = Search(_TEST_FILE_SUFFIX, fileinfo_cc.BaseName()) if matched_test_suffix: filename_cc = filename_cc[:-len(matched_test_suffix.group(1))] + filename_cc = filename_cc.replace('/public/', '/') filename_cc = filename_cc.replace('/internal/', '/') - if not filename_h.endswith('.h'): - return (False, '') - filename_h = filename_h[:-len('.h')] + filename_h = filename_h[:-(len(fileinfo_h.Extension()))] if filename_h.endswith('-inl'): filename_h = filename_h[:-len('-inl')] filename_h = filename_h.replace('/public/', '/') @@ -5508,7 +5765,7 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, # include_dict is modified during iteration, so we iterate over a copy of # the keys. - header_keys = include_dict.keys() + header_keys = list(include_dict.keys()) for header in header_keys: (same_module, common_path) = FilesBelongToSameModule(abs_filename, header) fullpath = common_path + header @@ -5520,11 +5777,13 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, # didn't include it in the .h file. # TODO(unknown): Do a better job of finding .h files so we are confident that # not having the .h file means there isn't one. - if filename.endswith('.cc') and not header_found: - return + if not header_found: + for extension in GetNonHeaderExtensions(): + if filename.endswith('.' + extension): + return # All the lines have been processed, report the errors found. - for required_header_unstripped in required: + for required_header_unstripped in sorted(required, key=required.__getitem__): template = required[required_header_unstripped][1] if required_header_unstripped.strip('<>"') not in include_dict: error(filename, required[required_header_unstripped][0], @@ -5663,11 +5922,9 @@ def IsBlockInNameSpace(nesting_state, is_forward_declaration): Whether or not the new block is directly in a namespace. """ if is_forward_declaration: - if len(nesting_state.stack) >= 1 and ( - isinstance(nesting_state.stack[-1], _NamespaceInfo)): - return True - else: - return False + return len(nesting_state.stack) >= 1 and ( + isinstance(nesting_state.stack[-1], _NamespaceInfo)) + return (len(nesting_state.stack) > 1 and nesting_state.stack[-1].check_namespace_indentation and @@ -5717,7 +5974,7 @@ def CheckItemIndentationInNamespace(filename, raw_lines_no_comments, linenum, def ProcessLine(filename, file_extension, clean_lines, line, include_state, function_state, nesting_state, error, - extra_check_functions=[]): + extra_check_functions=None): """Processes a single line in the file. Args: @@ -5756,8 +6013,9 @@ def ProcessLine(filename, file_extension, clean_lines, line, CheckMakePairUsesDeduction(filename, clean_lines, line, error) CheckRedundantVirtual(filename, clean_lines, line, error) CheckRedundantOverrideOrFinal(filename, clean_lines, line, error) - for check_fn in extra_check_functions: - check_fn(filename, clean_lines, line, error) + if extra_check_functions: + for check_fn in extra_check_functions: + check_fn(filename, clean_lines, line, error) def FlagCxx11Features(filename, clean_lines, linenum, error): """Flag those c++11 features that we only allow in certain places. @@ -5831,7 +6089,7 @@ def FlagCxx14Features(filename, clean_lines, linenum, error): def ProcessFileData(filename, file_extension, lines, error, - extra_check_functions=[]): + extra_check_functions=None): """Performs lint checks and reports any errors to the given error function. Args: @@ -5859,7 +6117,7 @@ def ProcessFileData(filename, file_extension, lines, error, RemoveMultiLineComments(filename, lines, error) clean_lines = CleansedLines(lines) - if file_extension == 'h': + if file_extension in GetHeaderExtensions(): CheckForHeaderGuard(filename, clean_lines, error) for line in xrange(clean_lines.NumLines()): @@ -5930,9 +6188,8 @@ def ProcessConfigOverrides(filename): if base_name: pattern = re.compile(val) if pattern.match(base_name): - sys.stderr.write('Ignoring "%s": file excluded by "%s". ' - 'File path component "%s" matches ' - 'pattern "%s"\n' % + _cpplint_state.PrintInfo('Ignoring "%s": file excluded by ' + '"%s". File path component "%s" matches pattern "%s"\n' % (filename, cfg_file, base_name, val)) return False elif name == 'linelength': @@ -5940,26 +6197,47 @@ def ProcessConfigOverrides(filename): try: _line_length = int(val) except ValueError: - sys.stderr.write('Line length must be numeric.') + _cpplint_state.PrintError('Line length must be numeric.') + elif name == 'extensions': + global _valid_extensions + try: + extensions = [ext.strip() for ext in val.split(',')] + _valid_extensions = set(extensions) + except ValueError: + sys.stderr.write('Extensions should be a comma-separated list of values;' + 'for example: extensions=hpp,cpp\n' + 'This could not be parsed: "%s"' % (val,)) + elif name == 'headers': + global _header_extensions + try: + extensions = [ext.strip() for ext in val.split(',')] + _header_extensions = set(extensions) + except ValueError: + sys.stderr.write('Extensions should be a comma-separated list of values;' + 'for example: extensions=hpp,cpp\n' + 'This could not be parsed: "%s"' % (val,)) + elif name == 'root': + global _root + _root = val else: - sys.stderr.write( + _cpplint_state.PrintError( 'Invalid configuration option (%s) in file %s\n' % (name, cfg_file)) except IOError: - sys.stderr.write( + _cpplint_state.PrintError( "Skipping config file '%s': Can't open for reading\n" % cfg_file) keep_looking = False # Apply all the accumulated filters in reverse order (top-level directory # config options having the least priority). - for filter in reversed(cfg_filters): - _AddFilters(filter) + for cfg_filter in reversed(cfg_filters): + _AddFilters(cfg_filter) return True -def ProcessFile(filename, vlevel, extra_check_functions=[]): +def ProcessFile(filename, vlevel, extra_check_functions=None): """Does google-lint on a single file. Args: @@ -6008,7 +6286,7 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): lf_lines.append(linenum + 1) except IOError: - sys.stderr.write( + _cpplint_state.PrintError( "Skipping input '%s': Can't open for reading\n" % filename) _RestoreFilters() return @@ -6018,9 +6296,9 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): # When reading from stdin, the extension is unknown, so no cpplint tests # should rely on the extension. - if filename != '-' and file_extension not in _valid_extensions: - sys.stderr.write('Ignoring %s; not a valid file name ' - '(%s)\n' % (filename, ', '.join(_valid_extensions))) + if filename != '-' and file_extension not in GetAllExtensions(): + _cpplint_state.PrintError('Ignoring %s; not a valid file name ' + '(%s)\n' % (filename, ', '.join(GetAllExtensions()))) else: ProcessFileData(filename, file_extension, lines, Error, extra_check_functions) @@ -6043,6 +6321,7 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): Error(filename, linenum, 'whitespace/newline', 1, 'Unexpected \\r (^M) found; better to use only \\n') + _cpplint_state.PrintInfo('Done processing %s\n' % filename) _RestoreFilters() @@ -6053,10 +6332,11 @@ def PrintUsage(message): message: The optional error message. """ sys.stderr.write(_USAGE) + if message: sys.exit('\nFATAL ERROR: ' + message) else: - sys.exit(1) + sys.exit(0) def PrintCategories(): @@ -6085,8 +6365,13 @@ def ParseArguments(args): 'filter=', 'logfile=', 'root=', + 'repository=', 'linelength=', - 'extensions=']) + 'extensions=', + 'exclude=', + 'headers=', + 'quiet', + 'recursive']) except getopt.GetoptError: PrintUsage('Invalid arguments.') @@ -6094,14 +6379,15 @@ def ParseArguments(args): output_format = _OutputFormat() filters = '' counting_style = '' + recursive = False for (opt, val) in opts: if opt == '--help': PrintUsage(None) elif opt == '--output': - if val not in ('emacs', 'vs7', 'eclipse', 'tap'): + if val not in ('emacs', 'vs7', 'eclipse', 'junit', 'tap'): PrintUsage( - 'The only allowed output formats are emacs, vs7, eclipse and tap.') + 'The only allowed output formats are emacs, vs7, eclipse, junit and tap.') output_format = val elif opt == '--verbose': verbosity = int(val) @@ -6116,24 +6402,50 @@ def ParseArguments(args): elif opt == '--root': global _root _root = val + elif opt == '--repository': + global _repository + _repository = val elif opt == '--linelength': global _line_length try: _line_length = int(val) except ValueError: PrintUsage('Line length must be digits.') + elif opt == '--exclude': + global _excludes + if not _excludes: + _excludes = set() + _excludes.update(glob.glob(val)) elif opt == '--extensions': global _valid_extensions try: _valid_extensions = set(val.split(',')) except ValueError: + PrintUsage('Extensions must be comma seperated list.') + elif opt == '--headers': + global _header_extensions + try: + _header_extensions = set(val.split(',')) + except ValueError: + PrintUsage('Extensions must be comma seperated list.') + elif opt == '--recursive': PrintUsage('Extensions must be comma separated list.') elif opt == '--logfile': + recursive = True + elif opt == '--quiet': logger.addHandler(logging.FileHandler(val, mode='wb')) + global _quiet + _quiet = True if not filenames: PrintUsage('No files were specified.') + if recursive: + filenames = _ExpandDirectories(filenames) + + if _excludes: + filenames = _FilterExcludedFiles(filenames) + _SetOutputFormat(output_format) _SetVerboseLevel(verbosity) _SetFilters(filters) @@ -6141,27 +6453,72 @@ def ParseArguments(args): return filenames +def _ExpandDirectories(filenames): + """Searches a list of filenames and replaces directories in the list with + all files descending from those directories. Files with extensions not in + the valid extensions list are excluded. + + Args: + filenames: A list of files or directories + + Returns: + A list of all files that are members of filenames or descended from a + directory in filenames + """ + expanded = set() + for filename in filenames: + if not os.path.isdir(filename): + expanded.add(filename) + continue + + for root, _, files in os.walk(filename): + for loopfile in files: + fullname = os.path.join(root, loopfile) + if fullname.startswith('.' + os.path.sep): + fullname = fullname[len('.' + os.path.sep):] + expanded.add(fullname) + + filtered = [] + for filename in expanded: + if os.path.splitext(filename)[1][1:] in GetAllExtensions(): + filtered.append(filename) + + return filtered + +def _FilterExcludedFiles(filenames): + """Filters out files listed in the --exclude command line switch. File paths + in the switch are evaluated relative to the current working directory + """ + exclude_paths = [os.path.abspath(f) for f in _excludes] + return [f for f in filenames if os.path.abspath(f) not in exclude_paths] def main(): filenames = ParseArguments(sys.argv[1:]) + backup_err = sys.stderr + try: + # Change stderr to write with replacement characters so we don't die + # if we try to print something containing non-ASCII characters. + sys.stderr = codecs.StreamReaderWriter(sys.stderr, + codecs.getreader('utf8'), + codecs.getwriter('utf8'), + 'replace') - # Change stderr to write with replacement characters so we don't die - # if we try to print something containing non-ASCII characters. - sys.stderr = codecs.StreamReaderWriter(sys.stderr, - codecs.getreader('utf8'), - codecs.getwriter('utf8'), - 'replace') + logger.addHandler(logging.StreamHandler(sys.stdout)) + logger.setLevel(logging.INFO) - logger.addHandler(logging.StreamHandler(sys.stdout)) - logger.setLevel(logging.INFO) + _cpplint_state.ResetErrorCounts() + for filename in filenames: + ProcessFile(filename.decode('utf-8'), _cpplint_state.verbose_level) + _cpplint_state.PrintErrorCounts() - if _cpplint_state.output_format == 'tap': - logger.info('TAP version 13') + if _cpplint_state.output_format == 'tap': + logger.info('TAP version 13') - _cpplint_state.ResetErrorCounts() - for filename in filenames: - ProcessFile(filename.decode('utf-8'), _cpplint_state.verbose_level) - _cpplint_state.PrintErrorCounts() + if _cpplint_state.output_format == 'junit': + sys.stderr.write(_cpplint_state.FormatJUnitXML()) + + finally: + sys.stderr = backup_err sys.exit(_cpplint_state.error_count > 0)