From 01e47236c846571b82a0136f79288a0b07916097 Mon Sep 17 00:00:00 2001 From: Alex Vakulenko Date: Fri, 6 May 2016 13:54:15 -0700 Subject: [PATCH] Update cpplint.py to r456 456 - Tweak lint to sometimes allow { on line following array initialization. 455 - Recognize more types. 454 - Try a bit harder to detect templated types. 453 - Changed error message wording on build/storage_class to be less ambiguous. 452 - Recognize more types to silence false positives for brace warnings. 451 - 450 - 449 - Ignore whitespace/tab for Linux Kernel files. 448 - Remove some more false positive lint warnings for int64{n}. 447 - Just one warning message for line length is enough, don't need two. 446 - Allow braced conversions to int16, uint64, etc. per the style guide. 445 - Fixed handling of backslash escapes for checking whether a "//" is quoted. 444 - Reduced dependency on hardcoded .cc extension. 443 - Disable single-arg constructor checks by default, since ClangTidy has the same check. We could just delete the check entirely, but it's slightly useful in places that can't run ClangTidy. 442 - Fix matching of macro names in CheckTrailingSemicolon to include digits. 441 - Deleted checks: - All checks for RValue references. - Spacing check around boolean &&, because those look like RValue references. A huge amount of code and effort were dedicated to tell RValue references apart from boolean expressions, and to differentiate allowed versus banned RValue references. But we still get regular false positives from this one check. Rather than making the check more complex than what it already is, it seemed safer to just delete it altogether for now, and have a different process for catching RValue references. 440 - Add rule to cpplint to throw error on empty if statement bodies without else clauses. If statement bodies with comments are not considered empty. 439 - Allow spaces before closing brace of namespace block so that namespaces inside of macro definitions (where the entire macro definition is indented) are not treated as errors. 438 - cpplint: fix a false positive on `new const int(x)`. 437 - Only check for function length when current line is inside a function. 436 - cpplint: Catch static `std::string` instances as well as those written as `string`. Previously users would sometimes work around the lint warning by changing their code to even worse code by adding the "std::" prefix. 435 - cpplint: Be a little smarter about warning on indentation. 434 - cpplint: Remove false positives on some functions returning string const&. 433 - cpplint: improve diagnostics of global/static string objects. 432 - Allow non-const reference parameters for iostream based types. 431 - Better handling of raw strings inside comments. 430 - 429 - 428 - 427 - Add support to CHECK_NOTNULL when checking if a member variable is initialized with itself. 426 - Allow suppressing specific warnings in C headers. 425 - Allow spaces before parens for inline assembly. 424 - Remove lint checks for {EXPECT,ASSERT}_.*_M. 423 - Allow comment lines of more than 80 characters if they contain a single "word" (without any spaces). 422 - cpplint: Warn on inclusion of C++14 headers. 421 - cpplint: recognize as a standard library header. 420 - Add to cpplint's list of C++ standard headers. 419 - Add cpplint check for tr1/ headers. We removed the nanny guards for these 418 - 417 - Update the styleguide and cpplint to allow unnamed parameters 416 - Remove the rule explicitly banning non-default move operations. 415 - Remove the check for explicit multi arg constructors 414 - Include clarity of lambda default captures in "pros" section, mention of capturing `this` and lambdas which escape the current scope in "cons". Soften the ban on default captures to a preference, with admonition against using them for capturing `this` or when they will escape the current scope. This is a fairly straightforward change with a brief inclusion of 2 problematic cases for implicit capture. As this is the style guide, I'm not sure how much more detail is appropriate. 413 - Fixed a bug by making regex in CleanseRawStrings match multiple raw strings on a single line in left-to-right order. 412 - Fixed false positive for classes derived using decltype() 411 - 410 - Recognize '1LL<<20' as numeric and don't flag it for spacing. --- cpplint/cpplint.py | 986 ++++++++++++++---------------------- cpplint/cpplint_unittest.py | 534 +++++++++++-------- 2 files changed, 696 insertions(+), 824 deletions(-) diff --git a/cpplint/cpplint.py b/cpplint/cpplint.py index 61bae45..9491320 100755 --- a/cpplint/cpplint.py +++ b/cpplint/cpplint.py @@ -177,6 +177,8 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] _ERROR_CATEGORIES = [ 'build/class', 'build/c++11', + 'build/c++14', + 'build/c++tr1', 'build/deprecated', 'build/endif_comment', 'build/explicit_make_pair', @@ -196,7 +198,6 @@ _ERROR_CATEGORIES = [ 'readability/check', 'readability/constructors', 'readability/fn_size', - 'readability/function', 'readability/inheritance', 'readability/multiline_comment', 'readability/multiline_string', @@ -227,6 +228,7 @@ _ERROR_CATEGORIES = [ 'whitespace/comma', 'whitespace/comments', 'whitespace/empty_conditional_body', + 'whitespace/empty_if_body', 'whitespace/empty_loop_body', 'whitespace/end_of_line', 'whitespace/ending_newline', @@ -245,6 +247,7 @@ _ERROR_CATEGORIES = [ # compatibility they may still appear in NOLINT comments. _LEGACY_ERROR_CATEGORIES = [ 'readability/streams', + 'readability/function', ] # The default state of the category filter. This is overridden by the --filter= @@ -253,6 +256,16 @@ _LEGACY_ERROR_CATEGORIES = [ # All entries here should start with a '-' or '+', as in the --filter= flag. _DEFAULT_FILTERS = ['-build/include_alpha'] +# The default list of categories suppressed for C (not C++) files. +_DEFAULT_C_SUPPRESSED_CATEGORIES = [ + 'readability/casting', + ] + +# The default list of categories suppressed for Linux Kernel files. +_DEFAULT_KERNEL_SUPPRESSED_CATEGORIES = [ + 'whitespace/tab', + ] + # We used to check for high-bit characters, but after much discussion we # decided those were OK, as long as they were in UTF-8 and didn't represent # hard-coded international strings, which belong in a separate i18n file. @@ -346,6 +359,7 @@ _CPP_HEADERS = frozenset([ 'random', 'ratio', 'regex', + 'scoped_allocator', 'set', 'sstream', 'stack', @@ -393,6 +407,19 @@ _CPP_HEADERS = frozenset([ 'cwctype', ]) +# Type names +_TYPES = re.compile( + r'^(?:' + # [dcl.type.simple] + r'(char(16_t|32_t)?)|wchar_t|' + r'bool|short|int|long|signed|unsigned|float|double|' + # [support.types] + r'(ptrdiff_t|size_t|max_align_t|nullptr_t)|' + # [cstdint.syn] + r'(u?int(_fast|_least)?(8|16|32|64)_t)|' + r'(u?int(max|ptr)_t)|' + r')$') + # These headers are excluded from [build/include] and [build/include_order] # checks: @@ -402,16 +429,18 @@ _CPP_HEADERS = frozenset([ _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)$' + +# Pattern that matches only complete whitespace, possibly across multiple lines. +_EMPTY_CONDITIONAL_BODY_PATTERN = re.compile(r'^\s*$', re.DOTALL) # Assertion macros. These are defined in base/logging.h and -# testing/base/gunit.h. Note that the _M versions need to come first -# for substring matching to work. +# testing/base/public/gunit.h. _CHECK_MACROS = [ 'DCHECK', 'CHECK', - 'EXPECT_TRUE_M', 'EXPECT_TRUE', - 'ASSERT_TRUE_M', 'ASSERT_TRUE', - 'EXPECT_FALSE_M', 'EXPECT_FALSE', - 'ASSERT_FALSE_M', 'ASSERT_FALSE', + 'EXPECT_TRUE', 'ASSERT_TRUE', + 'EXPECT_FALSE', 'ASSERT_FALSE', ] # Replacement macros for CHECK/DCHECK/EXPECT_TRUE/EXPECT_FALSE @@ -424,16 +453,12 @@ for op, replacement in [('==', 'EQ'), ('!=', 'NE'), _CHECK_REPLACEMENT['CHECK'][op] = 'CHECK_%s' % replacement _CHECK_REPLACEMENT['EXPECT_TRUE'][op] = 'EXPECT_%s' % replacement _CHECK_REPLACEMENT['ASSERT_TRUE'][op] = 'ASSERT_%s' % replacement - _CHECK_REPLACEMENT['EXPECT_TRUE_M'][op] = 'EXPECT_%s_M' % replacement - _CHECK_REPLACEMENT['ASSERT_TRUE_M'][op] = 'ASSERT_%s_M' % replacement for op, inv_replacement in [('==', 'NE'), ('!=', 'EQ'), ('>=', 'LT'), ('>', 'LE'), ('<=', 'GT'), ('<', 'GE')]: _CHECK_REPLACEMENT['EXPECT_FALSE'][op] = 'EXPECT_%s' % inv_replacement _CHECK_REPLACEMENT['ASSERT_FALSE'][op] = 'ASSERT_%s' % inv_replacement - _CHECK_REPLACEMENT['EXPECT_FALSE_M'][op] = 'EXPECT_%s_M' % inv_replacement - _CHECK_REPLACEMENT['ASSERT_FALSE_M'][op] = 'ASSERT_%s_M' % inv_replacement # Alternative tokens and their replacements. For full list, see section 2.5 # Alternative tokens [lex.digraph] in the C++ standard. @@ -482,6 +507,12 @@ _MATCH_ASM = re.compile(r'^\s*(?:asm|_asm|__asm|__asm__)' r'(?:\s+(volatile|__volatile__))?' r'\s*[{(]') +# Match strings that indicate we're working on a C (not C++) file. +_SEARCH_C_FILE = re.compile(r'\b(?:LINT_C_FILE|' + r'vim?:\s*.*(\s*|:)filetype=c(\s*|:|$))') + +# Match string that indicates we're working on a Linux Kernel file. +_SEARCH_KERNEL_FILE = re.compile(r'\b(?:LINT_KERNEL_FILE)') _regexp_compile_cache = {} @@ -501,8 +532,13 @@ _line_length = 80 # This is set by --extensions flag. _valid_extensions = set(['cc', 'h', 'cpp', 'cu', 'cuh']) +# {str, bool}: a map from error categories to booleans which indicate if the +# category should be suppressed for every line. +_global_error_suppressions = {} + + def ParseNolintSuppressions(filename, raw_line, linenum, error): - """Updates the global list of error-suppressions. + """Updates the global list of line error-suppressions. Parses any NOLINT comments on the current line, updating the global error_suppressions store. Reports an error if the NOLINT comment @@ -533,24 +569,45 @@ def ParseNolintSuppressions(filename, raw_line, linenum, error): 'Unknown NOLINT error category: %s' % category) +def ProcessGlobalSuppresions(lines): + """Updates the list of global error suppressions. + + Parses any lint directives in the file that have global effect. + + Args: + lines: An array of strings, each representing a line of the file, with the + last element being empty if the file is terminated with a newline. + """ + for line in lines: + if _SEARCH_C_FILE.search(line): + for category in _DEFAULT_C_SUPPRESSED_CATEGORIES: + _global_error_suppressions[category] = True + if _SEARCH_KERNEL_FILE.search(line): + for category in _DEFAULT_KERNEL_SUPPRESSED_CATEGORIES: + _global_error_suppressions[category] = True + + def ResetNolintSuppressions(): """Resets the set of NOLINT suppressions to empty.""" _error_suppressions.clear() + _global_error_suppressions.clear() def IsErrorSuppressedByNolint(category, linenum): """Returns true if the specified error category is suppressed on this line. Consults the global error_suppressions map populated by - ParseNolintSuppressions/ResetNolintSuppressions. + ParseNolintSuppressions/ProcessGlobalSuppresions/ResetNolintSuppressions. Args: category: str, the category of the error. linenum: int, the current line number. Returns: - bool, True iff the error should be suppressed due to a NOLINT comment. + bool, True iff the error should be suppressed due to a NOLINT comment or + global suppression. """ - return (linenum in _error_suppressions.get(category, set()) or + return (_global_error_suppressions.get(category, False) or + linenum in _error_suppressions.get(category, set()) or linenum in _error_suppressions.get(None, set())) @@ -589,6 +646,11 @@ def Search(pattern, s): return _regexp_compile_cache[pattern].search(s) +def _IsSourceExtension(s): + """File extension (excluding dot) matches a source file extension.""" + return s in ('c', 'cc', 'cpp', 'cxx') + + class _IncludeState(object): """Tracks line numbers for includes, and the order in which includes appear. @@ -944,6 +1006,9 @@ class _FunctionState(object): filename: The name of the current file. linenum: The number of the line to check. """ + if not self.in_a_function: + return + if Match(r'T(EST|est)', self.current_function): base_trigger = self._TEST_TRIGGER else: @@ -1014,12 +1079,13 @@ class FileInfo(object): # Not SVN <= 1.6? Try to find a git, hg, or svn top level directory by # searching up from the current path. - root_dir = os.path.dirname(fullname) - while (root_dir != os.path.dirname(root_dir) and - not os.path.exists(os.path.join(root_dir, ".git")) and - not os.path.exists(os.path.join(root_dir, ".hg")) and - not os.path.exists(os.path.join(root_dir, ".svn"))): - root_dir = os.path.dirname(root_dir) + root_dir = current_dir = os.path.dirname(fullname) + while current_dir != os.path.dirname(current_dir): + if (os.path.exists(os.path.join(current_dir, ".git")) or + os.path.exists(os.path.join(current_dir, ".hg")) or + os.path.exists(os.path.join(current_dir, ".svn"))): + root_dir = current_dir + current_dir = os.path.dirname(current_dir) if (os.path.exists(os.path.join(root_dir, ".git")) or os.path.exists(os.path.join(root_dir, ".hg")) or @@ -1058,7 +1124,7 @@ class FileInfo(object): def IsSource(self): """File has a source file extension.""" - return self.Extension()[1:] in ('c', 'cc', 'cpp', 'cxx') + return _IsSourceExtension(self.Extension()[1:]) def _ShouldPrintError(category, confidence, linenum): @@ -1204,8 +1270,18 @@ def CleanseRawStrings(raw_lines): while delimiter is None: # Look for beginning of a raw string. # See 2.14.15 [lex.string] for syntax. - matched = Match(r'^(.*)\b(?:R|u8R|uR|UR|LR)"([^\s\\()]*)\((.*)$', line) - if matched: + # + # Once we have matched a raw string, we check the prefix of the + # line to make sure that the line is not part of a single line + # comment. It's done this way because we remove raw strings + # before removing comments as opposed to removing comments + # before removing raw strings. This is because there are some + # cpplint checks that requires the comments to be preserved, but + # we don't want to check comments that are inside raw strings. + matched = Match(r'^(.*?)\b(?:R|u8R|uR|UR|LR)"([^\s\\()]*)\((.*)$', line) + if (matched and + not Match(r'^([^\'"]|\'(\\.|[^\'])*\'|"(\\.|[^"])*")*//', + matched.group(1))): delimiter = ')' + matched.group(2) + '"' end = matched.group(3).find(delimiter) @@ -1666,7 +1742,7 @@ def GetHeaderGuardCPPVariable(filename): filename = re.sub(r'/\.flymake/([^/]*)$', r'/\1', filename) # Replace 'c++' with 'cpp'. filename = filename.replace('C++', 'cpp').replace('c++', 'cpp') - + fileinfo = FileInfo(filename) file_path_from_root = fileinfo.RepositoryName() if _root: @@ -1776,11 +1852,11 @@ def CheckHeaderFileIncluded(filename, include_state, error): """Logs an error if a .cc file does not include its header.""" # Do not check test files - if filename.endswith('_test.cc') or filename.endswith('_unittest.cc'): + fileinfo = FileInfo(filename) + if Search(_TEST_FILE_SUFFIX, fileinfo.BaseName()): return - fileinfo = FileInfo(filename) - headerfile = filename[0:len(filename) - 2] + 'h' + headerfile = filename[0:len(filename) - len(fileinfo.Extension())] + '.h' if not os.path.exists(headerfile): return headername = FileInfo(headerfile).RepositoryName() @@ -1997,7 +2073,8 @@ def IsForwardClassDeclaration(clean_lines, linenum): class _BlockInfo(object): """Stores information about a generic block of code.""" - def __init__(self, seen_open_brace): + def __init__(self, linenum, seen_open_brace): + self.starting_linenum = linenum self.seen_open_brace = seen_open_brace self.open_parentheses = 0 self.inline_asm = _NO_ASM @@ -2046,17 +2123,16 @@ class _BlockInfo(object): class _ExternCInfo(_BlockInfo): """Stores information about an 'extern "C"' block.""" - def __init__(self): - _BlockInfo.__init__(self, True) + def __init__(self, linenum): + _BlockInfo.__init__(self, linenum, True) class _ClassInfo(_BlockInfo): """Stores information about a class.""" def __init__(self, name, class_or_struct, clean_lines, linenum): - _BlockInfo.__init__(self, False) + _BlockInfo.__init__(self, linenum, False) self.name = name - self.starting_linenum = linenum self.is_derived = False self.check_namespace_indentation = True if class_or_struct == 'struct': @@ -2124,9 +2200,8 @@ class _NamespaceInfo(_BlockInfo): """Stores information about a namespace.""" def __init__(self, name, linenum): - _BlockInfo.__init__(self, False) + _BlockInfo.__init__(self, linenum, False) self.name = name or '' - self.starting_linenum = linenum self.check_namespace_indentation = True def CheckEnd(self, filename, clean_lines, linenum, error): @@ -2145,7 +2220,7 @@ class _NamespaceInfo(_BlockInfo): # deciding what these nontrivial things are, so this check is # triggered by namespace size only, which works most of the time. if (linenum - self.starting_linenum < 10 - and not Match(r'};*\s*(//|/\*).*\bnamespace\b', line)): + and not Match(r'^\s*};*\s*(//|/\*).*\bnamespace\b', line)): return # Look for matching comment at end of namespace. @@ -2162,18 +2237,18 @@ class _NamespaceInfo(_BlockInfo): # expected namespace. if self.name: # Named namespace - if not Match((r'};*\s*(//|/\*).*\bnamespace\s+' + re.escape(self.name) + - r'[\*/\.\\\s]*$'), + if not Match((r'^\s*};*\s*(//|/\*).*\bnamespace\s+' + + re.escape(self.name) + r'[\*/\.\\\s]*$'), line): error(filename, linenum, 'readability/namespace', 5, 'Namespace should be terminated with "// namespace %s"' % self.name) else: # Anonymous namespace - if not Match(r'};*\s*(//|/\*).*\bnamespace[\*/\.\\\s]*$', line): + if not Match(r'^\s*};*\s*(//|/\*).*\bnamespace[\*/\.\\\s]*$', line): # If "// namespace anonymous" or "// anonymous namespace (more text)", # mention "// anonymous namespace" as an acceptable form - if Match(r'}.*\b(namespace anonymous|anonymous namespace)\b', line): + if Match(r'^\s*}.*\b(namespace anonymous|anonymous namespace)\b', line): error(filename, linenum, 'readability/namespace', 5, 'Anonymous namespace should be terminated with "// namespace"' ' or "// anonymous namespace"') @@ -2512,9 +2587,9 @@ class NestingState(object): if not self.SeenOpenBrace(): self.stack[-1].seen_open_brace = True elif Match(r'^extern\s*"[^"]*"\s*\{', line): - self.stack.append(_ExternCInfo()) + self.stack.append(_ExternCInfo(linenum)) else: - self.stack.append(_BlockInfo(True)) + self.stack.append(_BlockInfo(linenum, True)) if _MATCH_ASM.match(line): self.stack[-1].inline_asm = _BLOCK_ASM @@ -2626,7 +2701,8 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, r'\s+(register|static|extern|typedef)\b', line): error(filename, linenum, 'build/storage_class', 5, - 'Storage class (static, extern, typedef, etc) should be first.') + 'Storage-class specifier (static, extern, typedef, etc) should be ' + 'at the beginning of the declaration.') if Match(r'\s*#\s*endif\s*[^/\s]+', line): error(filename, linenum, 'build/endif_comment', 5, @@ -2665,9 +2741,7 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, base_classname = classinfo.name.split('::')[-1] # Look for single-argument constructors that aren't marked explicit. - # Technically a valid construct, but against style. Also look for - # non-single-argument constructors which are also technically valid, but - # strongly suggest something is wrong. + # Technically a valid construct, but against style. explicit_constructor_match = Match( r'\s+(?:inline\s+)?(explicit\s+)?(?:inline\s+)?%s\s*' r'\(((?:[^()]|\([^()]*\))*)\)' @@ -2728,10 +2802,6 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, if noarg_constructor: error(filename, linenum, 'runtime/explicit', 5, 'Zero-parameter constructors should not be marked explicit.') - else: - error(filename, linenum, 'runtime/explicit', 0, - 'Constructors that require multiple arguments ' - 'should not be marked explicit.') def CheckSpacingForFunctionCall(filename, clean_lines, linenum, error): @@ -2786,6 +2856,7 @@ def CheckSpacingForFunctionCall(filename, clean_lines, linenum, error): error(filename, linenum, 'whitespace/parens', 2, 'Extra space after (') if (Search(r'\w\s+\(', fncall) and + not Search(r'_{0,2}asm_{0,2}\s+_{0,2}volatile_{0,2}\s+\(', fncall) and not Search(r'#\s*define|typedef|using\s+\w+\s*=', fncall) and not Search(r'\w\s+\((\w+::)*\*\w+\)\(', fncall) and not Search(r'\bcase\s+\(', fncall)): @@ -2923,9 +2994,7 @@ def CheckComment(line, filename, linenum, next_line_start, error): commentpos = line.find('//') if commentpos != -1: # Check if the // may be in quotes. If so, ignore it - # Comparisons made explicit for clarity -- pylint: disable=g-explicit-bool-comparison - if (line.count('"', 0, commentpos) - - line.count('\\"', 0, commentpos)) % 2 == 0: # not in quotes + if re.sub(r'\\.', '', line[0:commentpos]).count('"') % 2 == 0: # Allow one space for new scopes, two spaces otherwise: if (not (Match(r'^.*{ *//', line) and next_line_start == commentpos) and ((commentpos >= 1 and @@ -3174,8 +3243,8 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # macro context and don't do any checks. This avoids false # positives. # - # Note that && is not included here. Those are checked separately - # in CheckRValueReference + # Note that && is not included here. This is because there are too + # many false positives due to RValue references. match = Search(r'[^<>=!\s](==|!=|<=|>=|\|\|)[^<>=!\s,;\)]', line) if match: error(filename, linenum, 'whitespace/operators', 3, @@ -3209,7 +3278,7 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # # We also allow operators following an opening parenthesis, since # those tend to be macros that deal with operators. - match = Search(r'(operator|[^\s(<])(?:L|UL|ULL|l|ul|ull)?<<([^\s,=<])', line) + match = Search(r'(operator|[^\s(<])(?:L|UL|LL|ULL|l|ul|ll|ull)?<<([^\s,=<])', line) if (match and not (match.group(1).isdigit() and match.group(2).isdigit()) and not (match.group(1) == 'operator' and match.group(2) == ';')): error(filename, linenum, 'whitespace/operators', 3, @@ -3313,22 +3382,90 @@ def CheckCommaSpacing(filename, clean_lines, linenum, error): 'Missing space after ;') -def CheckBracesSpacing(filename, clean_lines, linenum, error): +def _IsType(clean_lines, nesting_state, expr): + """Check if expression looks like a type name, returns true if so. + + Args: + clean_lines: A CleansedLines instance containing the file. + nesting_state: A NestingState instance which maintains information about + the current stack of nested blocks being parsed. + expr: The expression to check. + Returns: + True, if token looks like a type. + """ + # Keep only the last token in the expression + last_word = Match(r'^.*(\b\S+)$', expr) + if last_word: + token = last_word.group(1) + else: + token = expr + + # Match native types and stdint types + if _TYPES.match(token): + return True + + # Try a bit harder to match templated types. Walk up the nesting + # stack until we find something that resembles a typename + # declaration for what we are looking for. + typename_pattern = (r'\b(?:typename|class|struct)\s+' + re.escape(token) + + r'\b') + block_index = len(nesting_state.stack) - 1 + while block_index >= 0: + if isinstance(nesting_state.stack[block_index], _NamespaceInfo): + return False + + # Found where the opening brace is. We want to scan from this + # line up to the beginning of the function, minus a few lines. + # template + # class C + # : public ... { // start scanning here + last_line = nesting_state.stack[block_index].starting_linenum + + next_block_start = 0 + if block_index > 0: + next_block_start = nesting_state.stack[block_index - 1].starting_linenum + first_line = last_line + while first_line >= next_block_start: + if clean_lines.elided[first_line].find('template') >= 0: + break + first_line -= 1 + if first_line < next_block_start: + # Didn't find any "template" keyword before reaching the next block, + # there are probably no template things to check for this block + block_index -= 1 + continue + + # Look for typename in the specified range + for i in xrange(first_line, last_line + 1, 1): + if Search(typename_pattern, clean_lines.elided[i]): + return True + block_index -= 1 + + return False + + +def CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error): """Checks for horizontal spacing near commas. Args: filename: The name of the current file. clean_lines: A CleansedLines instance containing the file. linenum: The number of the line to check. + nesting_state: A NestingState instance which maintains information about + the current stack of nested blocks being parsed. error: The function to call with any errors found. """ line = clean_lines.elided[linenum] # Except after an opening paren, or after another opening brace (in case of # an initializer list, for instance), you should have spaces before your - # braces. And since you should never have braces at the beginning of a line, - # this is an easy test. + # braces when they are delimiting blocks, classes, namespaces etc. + # And since you should never have braces at the beginning of a line, + # this is an easy test. Except that braces used for initialization don't + # follow the same rule; we often don't want spaces before those. match = Match(r'^(.*[^ ({>]){', line) + if match: # Try a bit harder to check for brace initialization. This # happens in one of the following forms: @@ -3358,6 +3495,7 @@ def CheckBracesSpacing(filename, clean_lines, linenum, error): # There is a false negative with this approach if people inserted # spurious semicolons, e.g. "if (cond){};", but we will catch the # spurious semicolon with a separate check. + leading_text = match.group(1) (endline, endlinenum, endpos) = CloseExpression( clean_lines, linenum, len(match.group(1))) trailing_text = '' @@ -3366,7 +3504,11 @@ def CheckBracesSpacing(filename, clean_lines, linenum, error): for offset in xrange(endlinenum + 1, min(endlinenum + 3, clean_lines.NumLines() - 1)): trailing_text += clean_lines.elided[offset] - if not Match(r'^[\s}]*[{.;,)<>\]:]', trailing_text): + # We also suppress warnings for `uint64_t{expression}` etc., as the style + # guide recommends brace initialization for integral types to avoid + # overflow/truncation. + if (not Match(r'^[\s}]*[{.;,)<>\]:]', trailing_text) + and not _IsType(clean_lines, nesting_state, leading_text)): error(filename, linenum, 'whitespace/braces', 5, 'Missing space before {') @@ -3410,405 +3552,6 @@ def IsDecltype(clean_lines, linenum, column): return False -def IsTemplateParameterList(clean_lines, linenum, column): - """Check if the token ending on (linenum, column) is the end of template<>. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: the number of the line to check. - column: end column of the token to check. - Returns: - True if this token is end of a template parameter list, False otherwise. - """ - (_, startline, startpos) = ReverseCloseExpression( - clean_lines, linenum, column) - if (startpos > -1 and - Search(r'\btemplate\s*$', clean_lines.elided[startline][0:startpos])): - return True - return False - - -def IsRValueType(typenames, clean_lines, nesting_state, linenum, column): - """Check if the token ending on (linenum, column) is a type. - - Assumes that text to the right of the column is "&&" or a function - name. - - Args: - typenames: set of type names from template-argument-list. - clean_lines: A CleansedLines instance containing the file. - nesting_state: A NestingState instance which maintains information about - the current stack of nested blocks being parsed. - linenum: the number of the line to check. - column: end column of the token to check. - Returns: - True if this token is a type, False if we are not sure. - """ - prefix = clean_lines.elided[linenum][0:column] - - # Get one word to the left. If we failed to do so, this is most - # likely not a type, since it's unlikely that the type name and "&&" - # would be split across multiple lines. - match = Match(r'^(.*)(\b\w+|[>*)&])\s*$', prefix) - if not match: - return False - - # Check text following the token. If it's "&&>" or "&&," or "&&...", it's - # most likely a rvalue reference used inside a template. - suffix = clean_lines.elided[linenum][column:] - if Match(r'&&\s*(?:[>,]|\.\.\.)', suffix): - return True - - # Check for known types and end of templates: - # int&& variable - # vector&& variable - # - # Because this function is called recursively, we also need to - # recognize pointer and reference types: - # int* Function() - # int& Function() - if (match.group(2) in typenames or - match.group(2) in ['char', 'char16_t', 'char32_t', 'wchar_t', 'bool', - 'short', 'int', 'long', 'signed', 'unsigned', - 'float', 'double', 'void', 'auto', '>', '*', '&']): - return True - - # If we see a close parenthesis, look for decltype on the other side. - # decltype would unambiguously identify a type, anything else is - # probably a parenthesized expression and not a type. - if match.group(2) == ')': - return IsDecltype( - clean_lines, linenum, len(match.group(1)) + len(match.group(2)) - 1) - - # Check for casts and cv-qualifiers. - # match.group(1) remainder - # -------------- --------- - # const_cast< type&& - # const type&& - # type const&& - if Search(r'\b(?:const_cast\s*<|static_cast\s*<|dynamic_cast\s*<|' - r'reinterpret_cast\s*<|\w+\s)\s*$', - match.group(1)): - return True - - # Look for a preceding symbol that might help differentiate the context. - # These are the cases that would be ambiguous: - # match.group(1) remainder - # -------------- --------- - # Call ( expression && - # Declaration ( type&& - # sizeof ( type&& - # if ( expression && - # while ( expression && - # for ( type&& - # for( ; expression && - # statement ; type&& - # block { type&& - # constructor { expression && - start = linenum - line = match.group(1) - match_symbol = None - while start >= 0: - # We want to skip over identifiers and commas to get to a symbol. - # Commas are skipped so that we can find the opening parenthesis - # for function parameter lists. - match_symbol = Match(r'^(.*)([^\w\s,])[\w\s,]*$', line) - if match_symbol: - break - start -= 1 - line = clean_lines.elided[start] - - if not match_symbol: - # Probably the first statement in the file is an rvalue reference - return True - - if match_symbol.group(2) == '}': - # Found closing brace, probably an indicate of this: - # block{} type&& - return True - - if match_symbol.group(2) == ';': - # Found semicolon, probably one of these: - # for(; expression && - # statement; type&& - - # Look for the previous 'for(' in the previous lines. - before_text = match_symbol.group(1) - for i in xrange(start - 1, max(start - 6, 0), -1): - before_text = clean_lines.elided[i] + before_text - if Search(r'for\s*\([^{};]*$', before_text): - # This is the condition inside a for-loop - return False - - # Did not find a for-init-statement before this semicolon, so this - # is probably a new statement and not a condition. - return True - - if match_symbol.group(2) == '{': - # Found opening brace, probably one of these: - # block{ type&& = ... ; } - # constructor{ expression && expression } - - # Look for a closing brace or a semicolon. If we see a semicolon - # first, this is probably a rvalue reference. - line = clean_lines.elided[start][0:len(match_symbol.group(1)) + 1] - end = start - depth = 1 - while True: - for ch in line: - if ch == ';': - return True - elif ch == '{': - depth += 1 - elif ch == '}': - depth -= 1 - if depth == 0: - return False - end += 1 - if end >= clean_lines.NumLines(): - break - line = clean_lines.elided[end] - # Incomplete program? - return False - - if match_symbol.group(2) == '(': - # Opening parenthesis. Need to check what's to the left of the - # parenthesis. Look back one extra line for additional context. - before_text = match_symbol.group(1) - if linenum > 1: - before_text = clean_lines.elided[linenum - 1] + before_text - before_text = match_symbol.group(1) - - # Patterns that are likely to be types: - # [](type&& - # for (type&& - # sizeof(type&& - # operator=(type&& - # - if Search(r'(?:\]|\bfor|\bsizeof|\boperator\s*\S+\s*)\s*$', before_text): - return True - - # Patterns that are likely to be expressions: - # if (expression && - # while (expression && - # : initializer(expression && - # , initializer(expression && - # ( FunctionCall(expression && - # + FunctionCall(expression && - # + (expression && - # - # The last '+' represents operators such as '+' and '-'. - if Search(r'(?:\bif|\bwhile|[-+=%^(]*>)?\s*$', - match_symbol.group(1)) - if match_func: - # Check for constructors, which don't have return types. - if Search(r'\b(?:explicit|inline)$', match_func.group(1)): - return True - implicit_constructor = Match(r'\s*(\w+)\((?:const\s+)?(\w+)', prefix) - if (implicit_constructor and - implicit_constructor.group(1) == implicit_constructor.group(2)): - return True - return IsRValueType(typenames, clean_lines, nesting_state, linenum, - len(match_func.group(1))) - - # Nothing before the function name. If this is inside a block scope, - # this is probably a function call. - return not (nesting_state.previous_stack_top and - nesting_state.previous_stack_top.IsBlockInfo()) - - if match_symbol.group(2) == '>': - # Possibly a closing bracket, check that what's on the other side - # looks like the start of a template. - return IsTemplateParameterList( - clean_lines, start, len(match_symbol.group(1))) - - # Some other symbol, usually something like "a=b&&c". This is most - # likely not a type. - return False - - -def IsDeletedOrDefault(clean_lines, linenum): - """Check if current constructor or operator is deleted or default. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - Returns: - True if this is a deleted or default constructor. - """ - open_paren = clean_lines.elided[linenum].find('(') - if open_paren < 0: - return False - (close_line, _, close_paren) = CloseExpression( - clean_lines, linenum, open_paren) - if close_paren < 0: - return False - return Match(r'\s*=\s*(?:delete|default)\b', close_line[close_paren:]) - - -def IsRValueAllowed(clean_lines, linenum, typenames): - """Check if RValue reference is allowed on a particular line. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - typenames: set of type names from template-argument-list. - Returns: - True if line is within the region where RValue references are allowed. - """ - # Allow region marked by PUSH/POP macros - for i in xrange(linenum, 0, -1): - line = clean_lines.elided[i] - if Match(r'GOOGLE_ALLOW_RVALUE_REFERENCES_(?:PUSH|POP)', line): - if not line.endswith('PUSH'): - return False - for j in xrange(linenum, clean_lines.NumLines(), 1): - line = clean_lines.elided[j] - if Match(r'GOOGLE_ALLOW_RVALUE_REFERENCES_(?:PUSH|POP)', line): - return line.endswith('POP') - - # Allow operator= - line = clean_lines.elided[linenum] - if Search(r'\boperator\s*=\s*\(', line): - return IsDeletedOrDefault(clean_lines, linenum) - - # Allow constructors - match = Match(r'\s*(?:[\w<>]+::)*([\w<>]+)\s*::\s*([\w<>]+)\s*\(', line) - if match and match.group(1) == match.group(2): - return IsDeletedOrDefault(clean_lines, linenum) - if Search(r'\b(?:explicit|inline)\s+[\w<>]+\s*\(', line): - return IsDeletedOrDefault(clean_lines, linenum) - - if Match(r'\s*[\w<>]+\s*\(', line): - previous_line = 'ReturnType' - if linenum > 0: - previous_line = clean_lines.elided[linenum - 1] - if Match(r'^\s*$', previous_line) or Search(r'[{}:;]\s*$', previous_line): - return IsDeletedOrDefault(clean_lines, linenum) - - # Reject types not mentioned in template-argument-list - while line: - match = Match(r'^.*?(\w+)\s*&&(.*)$', line) - if not match: - break - if match.group(1) not in typenames: - return False - line = match.group(2) - - # All RValue types that were in template-argument-list should have - # been removed by now. Those were allowed, assuming that they will - # be forwarded. - # - # If there are no remaining RValue types left (i.e. types that were - # not found in template-argument-list), flag those as not allowed. - return line.find('&&') < 0 - - -def GetTemplateArgs(clean_lines, linenum): - """Find list of template arguments associated with this function declaration. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: Line number containing the start of the function declaration, - usually one line after the end of the template-argument-list. - Returns: - Set of type names, or empty set if this does not appear to have - any template parameters. - """ - # Find start of function - func_line = linenum - while func_line > 0: - line = clean_lines.elided[func_line] - if Match(r'^\s*$', line): - return set() - if line.find('(') >= 0: - break - func_line -= 1 - if func_line == 0: - return set() - - # Collapse template-argument-list into a single string - argument_list = '' - match = Match(r'^(\s*template\s*)<', clean_lines.elided[func_line]) - if match: - # template-argument-list on the same line as function name - start_col = len(match.group(1)) - _, end_line, end_col = CloseExpression(clean_lines, func_line, start_col) - if end_col > -1 and end_line == func_line: - start_col += 1 # Skip the opening bracket - argument_list = clean_lines.elided[func_line][start_col:end_col] - - elif func_line > 1: - # template-argument-list one line before function name - match = Match(r'^(.*)>\s*$', clean_lines.elided[func_line - 1]) - if match: - end_col = len(match.group(1)) - _, start_line, start_col = ReverseCloseExpression( - clean_lines, func_line - 1, end_col) - if start_col > -1: - start_col += 1 # Skip the opening bracket - while start_line < func_line - 1: - argument_list += clean_lines.elided[start_line][start_col:] - start_col = 0 - start_line += 1 - argument_list += clean_lines.elided[func_line - 1][start_col:end_col] - - if not argument_list: - return set() - - # Extract type names - typenames = set() - while True: - match = Match(r'^[,\s]*(?:typename|class)(?:\.\.\.)?\s+(\w+)(.*)$', - argument_list) - if not match: - break - typenames.add(match.group(1)) - argument_list = match.group(2) - return typenames - - -def CheckRValueReference(filename, clean_lines, linenum, nesting_state, error): - """Check for rvalue references. - - Args: - filename: The name of the current file. - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - nesting_state: A NestingState instance which maintains information about - the current stack of nested blocks being parsed. - error: The function to call with any errors found. - """ - # Find lines missing spaces around &&. - # TODO(unknown): currently we don't check for rvalue references - # with spaces surrounding the && to avoid false positives with - # boolean expressions. - line = clean_lines.elided[linenum] - match = Match(r'^(.*\S)&&', line) - if not match: - match = Match(r'(.*)&&\S', line) - if (not match) or '(&&)' in line or Search(r'\boperator\s*$', match.group(1)): - return - - # Either poorly formed && or an rvalue reference, check the context - # to get a more accurate error message. Mostly we want to determine - # if what's to the left of "&&" is a type or not. - typenames = GetTemplateArgs(clean_lines, linenum) - and_pos = len(match.group(1)) - if IsRValueType(typenames, clean_lines, nesting_state, linenum, and_pos): - if not IsRValueAllowed(clean_lines, linenum, typenames): - error(filename, linenum, 'build/c++11', 3, - 'RValue references are an unapproved C++ feature.') - else: - error(filename, linenum, 'whitespace/operators', 3, - 'Missing spaces around &&') - - def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error): """Checks for additional blank line issues related to sections. @@ -3906,10 +3649,13 @@ def CheckBraces(filename, clean_lines, linenum, error): # used for brace initializers inside function calls. We don't detect this # perfectly: we just don't complain if the last non-whitespace character on # the previous non-blank line is ',', ';', ':', '(', '{', or '}', or if the - # previous line starts a preprocessor block. + # previous line starts a preprocessor block. We also allow a brace on the + # following line if it is part of an array initialization and would not fit + # within the 80 character limit of the preceding line. prevline = GetPreviousNonBlankLine(clean_lines, linenum)[0] if (not Search(r'[,;:}{(]\s*$', prevline) and - not Match(r'\s*#', prevline)): + not Match(r'\s*#', prevline) and + not (GetLineWidth(prevline) > _line_length - 2 and '[]' in prevline)): error(filename, linenum, 'whitespace/braces', 4, '{ should almost always be at the end of the previous line') @@ -4085,13 +3831,14 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): # In addition to macros, we also don't want to warn on # - Compound literals # - Lambdas - # - alignas specifier with anonymous structs: + # - alignas specifier with anonymous structs + # - decltype closing_brace_pos = match.group(1).rfind(')') opening_parenthesis = ReverseCloseExpression( clean_lines, linenum, closing_brace_pos) if opening_parenthesis[2] > -1: line_prefix = opening_parenthesis[0][0:opening_parenthesis[2]] - macro = Search(r'\b([A-Z_]+)\s*$', line_prefix) + macro = Search(r'\b([A-Z_][A-Z0-9_]*)\s*$', line_prefix) func = Match(r'^(.*\])\s*$', line_prefix) if ((macro and macro.group(1) not in ( @@ -4100,6 +3847,7 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): 'LOCKS_EXCLUDED', 'INTERFACE_DEF')) or (func and not Search(r'\boperator\s*\[\s*\]', func.group(1))) or Search(r'\b(?:struct|union)\s+alignas\s*$', line_prefix) or + Search(r'\bdecltype$', line_prefix) or Search(r'\s+=\s*$', line_prefix)): match = None if (match and @@ -4159,7 +3907,7 @@ def CheckEmptyBlockBody(filename, clean_lines, linenum, error): line = clean_lines.elided[linenum] matched = Match(r'\s*(for|while|if)\s*\(', line) if matched: - # Find the end of the conditional expression + # Find the end of the conditional expression. (end_line, end_linenum, end_pos) = CloseExpression( clean_lines, linenum, line.find('(')) @@ -4174,6 +3922,75 @@ def CheckEmptyBlockBody(filename, clean_lines, linenum, error): error(filename, end_linenum, 'whitespace/empty_loop_body', 5, 'Empty loop bodies should use {} or continue') + # Check for if statements that have completely empty bodies (no comments) + # and no else clauses. + if end_pos >= 0 and matched.group(1) == 'if': + # Find the position of the opening { for the if statement. + # Return without logging an error if it has no brackets. + opening_linenum = end_linenum + opening_line_fragment = end_line[end_pos:] + # Loop until EOF or find anything that's not whitespace or opening {. + while not Search(r'^\s*\{', opening_line_fragment): + if Search(r'^(?!\s*$)', opening_line_fragment): + # Conditional has no brackets. + return + opening_linenum += 1 + if opening_linenum == len(clean_lines.elided): + # Couldn't find conditional's opening { or any code before EOF. + return + opening_line_fragment = clean_lines.elided[opening_linenum] + # Set opening_line (opening_line_fragment may not be entire opening line). + opening_line = clean_lines.elided[opening_linenum] + + # Find the position of the closing }. + opening_pos = opening_line_fragment.find('{') + if opening_linenum == end_linenum: + # We need to make opening_pos relative to the start of the entire line. + opening_pos += end_pos + (closing_line, closing_linenum, closing_pos) = CloseExpression( + clean_lines, opening_linenum, opening_pos) + if closing_pos < 0: + return + + # Now construct the body of the conditional. This consists of the portion + # of the opening line after the {, all lines until the closing line, + # and the portion of the closing line before the }. + if (clean_lines.raw_lines[opening_linenum] != + CleanseComments(clean_lines.raw_lines[opening_linenum])): + # Opening line ends with a comment, so conditional isn't empty. + return + if closing_linenum > opening_linenum: + # Opening line after the {. Ignore comments here since we checked above. + body = 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]) + # 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) + else: + # If statement has brackets and fits on a single line. + body = opening_line[opening_pos+1:closing_pos-1] + + # Check if the body is empty + if not _EMPTY_CONDITIONAL_BODY_PATTERN.search(body): + return + # The body is empty. Now make sure there's not an else clause. + current_linenum = closing_linenum + current_line_fragment = closing_line[closing_pos:] + # Loop until EOF or find anything that's not whitespace or else clause. + while Search(r'^\s*$|^(?=\s*else)', current_line_fragment): + if Search(r'^(?=\s*else)', current_line_fragment): + # Found an else clause, so don't log an error. + return + current_linenum += 1 + if current_linenum == len(clean_lines.elided): + break + current_line_fragment = clean_lines.elided[current_linenum] + + # The body is empty and there's no else clause until EOF or other code. + error(filename, end_linenum, 'whitespace/empty_if_body', 4, + ('If statement had no body and no else clause')) + def FindCheckMacro(line): """Find a replaceable CHECK-like macro. @@ -4393,6 +4210,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # raw strings, raw_lines = clean_lines.lines_without_raw_strings line = raw_lines[linenum] + prev = raw_lines[linenum - 1] if linenum > 0 else '' if line.find('\t') != -1: error(filename, linenum, 'whitespace/tab', 1, @@ -4416,19 +4234,24 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, cleansed_line = clean_lines.elided[linenum] while initial_spaces < len(line) and line[initial_spaces] == ' ': initial_spaces += 1 - if line and line[-1].isspace(): - error(filename, linenum, 'whitespace/end_of_line', 4, - 'Line ends in whitespace. Consider deleting these extra spaces.') # There are certain situations we allow one space, notably for # section labels, and also lines containing multi-line raw strings. - elif ((initial_spaces == 1 or initial_spaces == 3) and - not Match(scope_or_label_pattern, cleansed_line) and - not (clean_lines.raw_lines[linenum] != line and - Match(r'^\s*""', line))): + # We also don't check for lines that look like continuation lines + # (of lines ending in double quotes, commas, equals, or angle brackets) + # because the rules for how to indent those are non-trivial. + if (not Search(r'[",=><] *$', prev) and + (initial_spaces == 1 or initial_spaces == 3) and + not Match(scope_or_label_pattern, cleansed_line) and + not (clean_lines.raw_lines[linenum] != line and + Match(r'^\s*""', line))): error(filename, linenum, 'whitespace/indent', 3, 'Weird number of spaces at line-start. ' 'Are you using a 2-space indent?') + if line and line[-1].isspace(): + error(filename, linenum, 'whitespace/end_of_line', 4, + 'Line ends in whitespace. Consider deleting these extra spaces.') + # Check if the line is a header guard. is_header_guard = False if file_extension == 'h': @@ -4447,14 +4270,10 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # developers fault. 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)): line_width = GetLineWidth(line) - extended_length = int((_line_length * 1.25)) - if line_width > extended_length: - error(filename, linenum, 'whitespace/line_length', 4, - 'Lines should very rarely be longer than %i characters' % - extended_length) - elif line_width > _line_length: + if line_width > _line_length: error(filename, linenum, 'whitespace/line_length', 2, 'Lines should be <= %i characters long' % _line_length) @@ -4479,9 +4298,8 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, CheckOperatorSpacing(filename, clean_lines, linenum, error) CheckParenthesisSpacing(filename, clean_lines, linenum, error) CheckCommaSpacing(filename, clean_lines, linenum, error) - CheckBracesSpacing(filename, clean_lines, linenum, error) + CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error) CheckSpacingForFunctionCall(filename, clean_lines, linenum, error) - CheckRValueReference(filename, clean_lines, linenum, nesting_state, error) CheckCheck(filename, clean_lines, linenum, error) CheckAltTokens(filename, clean_lines, linenum, error) classinfo = nesting_state.InnermostClass() @@ -4525,23 +4343,6 @@ def _DropCommonSuffixes(filename): return os.path.splitext(filename)[0] -def _IsTestFilename(filename): - """Determines if the given filename has a suffix that identifies it as a test. - - Args: - filename: The input filename. - - Returns: - True if 'filename' looks like a test, False otherwise. - """ - if (filename.endswith('_test.cc') or - filename.endswith('_unittest.cc') or - filename.endswith('_regtest.cc')): - return True - else: - return False - - def _ClassifyInclude(fileinfo, include, is_system): """Figures out what kind of header 'include' is. @@ -4756,6 +4557,9 @@ _RE_PATTERN_REF_PARAM = re.compile( _RE_PATTERN_CONST_REF_PARAM = ( r'(?:.*\s*\bconst\s*&\s*' + _RE_PATTERN_IDENT + r'|const\s+' + _RE_PATTERN_TYPE + r'\s*&\s*' + _RE_PATTERN_IDENT + r')') +# Stream types. +_RE_PATTERN_REF_STREAM_PARAM = ( + r'(?:.*stream\s*&\s*' + _RE_PATTERN_IDENT + r')') def CheckLanguage(filename, clean_lines, linenum, file_extension, @@ -4794,7 +4598,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, # Make Windows paths like Unix. fullname = os.path.abspath(filename).replace('\\', '/') - + # Perform other checks now that we are sure that this is not an include line CheckCasts(filename, clean_lines, linenum, error) CheckGlobalStatic(filename, clean_lines, linenum, error) @@ -4933,9 +4737,13 @@ def CheckGlobalStatic(filename, clean_lines, linenum, error): # Check for people declaring static/global STL strings at the top level. # This is dangerous because the C++ language does not guarantee that - # globals with constructors are initialized before the first access. + # globals with constructors are initialized before the first access, and + # also because globals can be destroyed when some threads are still running. + # TODO(unknown): Generalize this to also find static unique_ptr instances. + # TODO(unknown): File bugs for clang-tidy to find these. match = Match( - r'((?:|static +)(?:|const +))string +([a-zA-Z0-9_:]+)\b(.*)', + r'((?:|static +)(?:|const +))(?::*std::)?string( +const)? +' + r'([a-zA-Z0-9_:]+)\b(.*)', line) # Remove false positives: @@ -4955,15 +4763,20 @@ def CheckGlobalStatic(filename, clean_lines, linenum, error): # matching identifiers. # string Class::operator*() if (match and - not Search(r'\bstring\b(\s+const)?\s*\*\s*(const\s+)?\w', line) and + not Search(r'\bstring\b(\s+const)?\s*[\*\&]\s*(const\s+)?\w', line) and not Search(r'\boperator\W', line) and - not Match(r'\s*(<.*>)?(::[a-zA-Z0-9_]+)*\s*\(([^"]|$)', match.group(3))): - error(filename, linenum, 'runtime/string', 4, - 'For a static/global string constant, use a C style string instead: ' - '"%schar %s[]".' % - (match.group(1), match.group(2))) + not Match(r'\s*(<.*>)?(::[a-zA-Z0-9_]+)*\s*\(([^"]|$)', match.group(4))): + if Search(r'\bconst\b', line): + error(filename, linenum, 'runtime/string', 4, + 'For a static/global string constant, use a C style string ' + 'instead: "%schar%s %s[]".' % + (match.group(1), match.group(2) or '', match.group(3))) + else: + error(filename, linenum, 'runtime/string', 4, + 'Static/global string variables are not permitted.') - if Search(r'\b([A-Za-z0-9_]*_)\(\1\)', line): + if (Search(r'\b([A-Za-z0-9_]*_)\(\1\)', line) or + Search(r'\b([A-Za-z0-9_]*_)\(CHECK_NOTNULL\(\1\)\)', line)): error(filename, linenum, 'runtime/init', 4, 'You seem to be initializing a member variable with itself.') @@ -5208,7 +5021,8 @@ def CheckForNonConstReference(filename, clean_lines, linenum, decls = ReplaceAll(r'{[^}]*}', ' ', line) # exclude function body for parameter in re.findall(_RE_PATTERN_REF_PARAM, decls): - if not Match(_RE_PATTERN_CONST_REF_PARAM, parameter): + if (not Match(_RE_PATTERN_CONST_REF_PARAM, parameter) and + not Match(_RE_PATTERN_REF_STREAM_PARAM, parameter)): error(filename, linenum, 'runtime/references', 2, 'Is this a non-const reference? ' 'If so, make const or use a pointer: ' + @@ -5231,7 +5045,7 @@ def CheckCasts(filename, clean_lines, linenum, error): # Parameterless conversion functions, such as bool(), are allowed as they are # probably a member operator declaration or default constructor. match = Search( - r'(\bnew\s+|\S<\s*(?:const\s+)?)?\b' + r'(\bnew\s+(?:const\s+)?|\S<\s*(?:const\s+)?)?\b' r'(int|float|double|bool|char|int32|uint32|int64|uint64)' r'(\([^)].*)', line) expecting_function = ExpectingFunctionArgs(clean_lines, linenum) @@ -5372,63 +5186,12 @@ def CheckCStyleCast(filename, clean_lines, linenum, cast_type, pattern, error): if context.endswith(' operator++') or context.endswith(' operator--'): return False - # A single unnamed argument for a function tends to look like old - # style cast. If we see those, don't issue warnings for deprecated - # casts, instead issue warnings for unnamed arguments where - # appropriate. - # - # These are things that we want warnings for, since the style guide - # explicitly require all parameters to be named: - # Function(int); - # Function(int) { - # ConstMember(int) const; - # ConstMember(int) const { - # ExceptionMember(int) throw (...); - # ExceptionMember(int) throw (...) { - # PureVirtual(int) = 0; - # [](int) -> bool { - # - # These are functions of some sort, where the compiler would be fine - # if they had named parameters, but people often omit those - # identifiers to reduce clutter: - # (FunctionPointer)(int); - # (FunctionPointer)(int) = value; - # Function((function_pointer_arg)(int)) - # Function((function_pointer_arg)(int), int param) - # ; - # <(FunctionPointerTemplateArgument)(int)>; + # A single unnamed argument for a function tends to look like old style cast. + # If we see those, don't issue warnings for deprecated casts. remainder = line[match.end(0):] if Match(r'^\s*(?:;|const\b|throw\b|final\b|override\b|[=>{),]|->)', remainder): - # Looks like an unnamed parameter. - - # Don't warn on any kind of template arguments. - if Match(r'^\s*>', remainder): - return False - - # Don't warn on assignments to function pointers, but keep warnings for - # unnamed parameters to pure virtual functions. Note that this pattern - # will also pass on assignments of "0" to function pointers, but the - # preferred values for those would be "nullptr" or "NULL". - matched_zero = Match(r'^\s=\s*(\S+)\s*;', remainder) - if matched_zero and matched_zero.group(1) != '0': - return False - - # Don't warn on function pointer declarations. For this we need - # to check what came before the "(type)" string. - if Match(r'.*\)\s*$', line[0:match.start(0)]): - return False - - # Don't warn if the parameter is named with block comments, e.g.: - # Function(int /*unused_param*/); - raw_line = clean_lines.raw_lines[linenum] - if '/*' in raw_line: - return False - - # Passed all filters, issue warning here. - error(filename, linenum, 'readability/function', 3, - 'All parameters should be named in a function') - return True + return False # At this point, all that should be left is actual casts. error(filename, linenum, 'readability/casting', 4, @@ -5498,18 +5261,26 @@ _HEADERS_CONTAINING_TEMPLATES = ( ('', ('slist',)), ) +_HEADERS_MAYBE_TEMPLATES = ( + ('', ('copy', 'max', 'min', 'min_element', 'sort', + 'transform', + )), + ('', ('swap',)), + ) + _RE_PATTERN_STRING = re.compile(r'\bstring\b') -_re_pattern_algorithm_header = [] -for _template in ('copy', 'max', 'min', 'min_element', 'sort', 'swap', - 'transform'): - # Match max(..., ...), max(..., ...), but not foo->max, foo.max or - # type::max(). - _re_pattern_algorithm_header.append( - (re.compile(r'[^>.]\b' + _template + r'(<.*?>)?\([^\)]'), - _template, - '')) +_re_pattern_headers_maybe_templates = [] +for _header, _templates in _HEADERS_MAYBE_TEMPLATES: + for _template in _templates: + # Match max(..., ...), max(..., ...), but not foo->max, foo.max or + # type::max(). + _re_pattern_headers_maybe_templates.append( + (re.compile(r'[^>.]\b' + _template + r'(<.*?>)?\([^\)]'), + _template, + _header)) +# Other scripts may reach in and modify this pattern. _re_pattern_templates = [] for _header, _templates in _HEADERS_CONTAINING_TEMPLATES: for _template in _templates: @@ -5549,13 +5320,13 @@ def FilesBelongToSameModule(filename_cc, filename_h): string: the additional prefix needed to open the header file. """ - if not filename_cc.endswith('.cc'): + fileinfo = FileInfo(filename_cc) + if not fileinfo.IsSource(): return (False, '') - filename_cc = filename_cc[:-len('.cc')] - if filename_cc.endswith('_unittest'): - filename_cc = filename_cc[:-len('_unittest')] - elif filename_cc.endswith('_test'): - filename_cc = filename_cc[:-len('_test')] + filename_cc = filename_cc[:-len(fileinfo.Extension())] + matched_test_suffix = Search(_TEST_FILE_SUFFIX, fileinfo.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/', '/') @@ -5636,7 +5407,7 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, if prefix.endswith('std::') or not prefix.endswith('::'): required[''] = (linenum, 'string') - for pattern, template, header in _re_pattern_algorithm_header: + for pattern, template, header in _re_pattern_headers_maybe_templates: if pattern.search(line): required[header] = (linenum, template) @@ -5719,31 +5490,6 @@ def CheckMakePairUsesDeduction(filename, clean_lines, linenum, error): ' OR use pair directly OR if appropriate, construct a pair directly') -def CheckDefaultLambdaCaptures(filename, clean_lines, linenum, error): - """Check that default lambda captures are not used. - - Args: - filename: The name of the current file. - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - error: The function to call with any errors found. - """ - line = clean_lines.elided[linenum] - - # A lambda introducer specifies a default capture if it starts with "[=" - # or if it starts with "[&" _not_ followed by an identifier. - match = Match(r'^(.*)\[\s*(?:=|&[^\w])', line) - if match: - # Found a potential error, check what comes after the lambda-introducer. - # If it's not open parenthesis (for lambda-declarator) or open brace - # (for compound-statement), it's not a lambda. - line, _, pos = CloseExpression(clean_lines, linenum, len(match.group(1))) - if pos >= 0 and Match(r'^\s*[{(]', line[pos:]): - error(filename, linenum, 'build/c++11', - 4, # 4 = high confidence - 'Default lambda captures are an unapproved C++ feature.') - - def CheckRedundantVirtual(filename, clean_lines, linenum, error): """Check if line contains a redundant "virtual" function-specifier. @@ -5942,7 +5688,6 @@ def ProcessLine(filename, file_extension, clean_lines, line, CheckPosixThreading(filename, clean_lines, line, error) CheckInvalidIncrement(filename, clean_lines, line, error) CheckMakePairUsesDeduction(filename, clean_lines, line, error) - CheckDefaultLambdaCaptures(filename, clean_lines, line, error) CheckRedundantVirtual(filename, clean_lines, line, error) CheckRedundantOverrideOrFinal(filename, clean_lines, line, error) for check_fn in extra_check_functions: @@ -5959,8 +5704,14 @@ def FlagCxx11Features(filename, clean_lines, linenum, error): """ line = clean_lines.elided[linenum] - # Flag unapproved C++11 headers. include = Match(r'\s*#\s*include\s+[<"]([^<"]+)[">]', line) + + # Flag unapproved C++ TR1 headers. + if include and include.group(1).startswith('tr1/'): + error(filename, linenum, 'build/c++tr1', 5, + ('C++ TR1 headers such as <%s> are unapproved.') % include.group(1)) + + # Flag unapproved C++11 headers. if include and include.group(1) in ('cfenv', 'condition_variable', 'fenv.h', @@ -5994,6 +5745,25 @@ def FlagCxx11Features(filename, clean_lines, linenum, error): 'they may let you use it.') % top_name) +def FlagCxx14Features(filename, clean_lines, linenum, error): + """Flag those C++14 features that we restrict. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] + + include = Match(r'\s*#\s*include\s+[<"]([^<"]+)[">]', line) + + # Flag unapproved C++14 headers. + if include and include.group(1) in ('scoped_allocator', 'shared_mutex'): + error(filename, linenum, 'build/c++14', 5, + ('<%s> is an unapproved C++14 header.') % include.group(1)) + + def ProcessFileData(filename, file_extension, lines, error, extra_check_functions=[]): """Performs lint checks and reports any errors to the given error function. @@ -6019,7 +5789,7 @@ def ProcessFileData(filename, file_extension, lines, error, ResetNolintSuppressions() CheckForCopyright(filename, lines, error) - + ProcessGlobalSuppresions(lines) RemoveMultiLineComments(filename, lines, error) clean_lines = CleansedLines(lines) @@ -6034,9 +5804,9 @@ def ProcessFileData(filename, file_extension, lines, error, nesting_state.CheckCompletedBlocks(filename, error) CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error) - + # Check that the .cc file has included its header if it exists. - if file_extension == 'cc': + if _IsSourceExtension(file_extension): CheckHeaderFileIncluded(filename, include_state, error) # We check here rather than inside ProcessLine so that we see raw diff --git a/cpplint/cpplint_unittest.py b/cpplint/cpplint_unittest.py index 8d9eae6..fd059fb 100755 --- a/cpplint/cpplint_unittest.py +++ b/cpplint/cpplint_unittest.py @@ -290,6 +290,14 @@ class CpplintTest(CpplintTestBase): results = self.GetNamespaceResults(lines) self.assertEquals(results, '') + def testWhitespaceBeforeNamespace(self): + lines = [' namespace Test {', + ' void foo() { }', + ' } // namespace Test'] + + results = self.GetNamespaceResults(lines) + self.assertEquals(results, '') + def testFalsePositivesNoError(self): lines = ['namespace Test {', 'struct OuterClass {', @@ -362,13 +370,23 @@ class CpplintTest(CpplintTestBase): '// Hello', '') self.TestLint( - '// ' + 'x' * 80, + '// x' + ' x' * 40, 'Lines should be <= 80 characters long' ' [whitespace/line_length] [2]') self.TestLint( - '// ' + 'x' * 100, - 'Lines should very rarely be longer than 100 characters' - ' [whitespace/line_length] [4]') + '// x' + ' x' * 50, + 'Lines should be <= 80 characters long' + ' [whitespace/line_length] [2]') + self.TestLint( + '// //some/path/to/f' + ('i' * 100) + 'le', + '') + self.TestLint( + '// //some/path/to/f' + ('i' * 100) + 'le', + '') + self.TestLint( + '// //some/path/to/f' + ('i' * 50) + 'le and some comments', + 'Lines should be <= 80 characters long' + ' [whitespace/line_length] [2]') self.TestLint( '// http://g' + ('o' * 100) + 'gle.com/', '') @@ -445,6 +463,64 @@ class CpplintTest(CpplintTestBase): ''], error_collector) self.assertEquals('', error_collector.Results()) + # LINT_C_FILE silences cast warnings for entire file. + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('test.h', 'h', + ['// Copyright 2014 Your Company.', + '// NOLINT(build/header_guard)', + 'int64 a = (uint64) 65;', + '// LINT_C_FILE', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) + # Vim modes silence cast warnings for entire file. + for modeline in ['vi:filetype=c', + 'vi:sw=8 filetype=c', + 'vi:sw=8 filetype=c ts=8', + 'vi: filetype=c', + 'vi: sw=8 filetype=c', + 'vi: sw=8 filetype=c ts=8', + 'vim:filetype=c', + 'vim:sw=8 filetype=c', + 'vim:sw=8 filetype=c ts=8', + 'vim: filetype=c', + 'vim: sw=8 filetype=c', + 'vim: sw=8 filetype=c ts=8', + 'vim: set filetype=c:', + 'vim: set sw=8 filetype=c:', + 'vim: set sw=8 filetype=c ts=8:', + 'vim: set filetype=c :', + 'vim: set sw=8 filetype=c :', + 'vim: set sw=8 filetype=c ts=8 :', + 'vim: se filetype=c:', + 'vim: se sw=8 filetype=c:', + 'vim: se sw=8 filetype=c ts=8:', + 'vim: se filetype=c :', + 'vim: se sw=8 filetype=c :', + 'vim: se sw=8 filetype=c ts=8 :']: + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('test.h', 'h', + ['// Copyright 2014 Your Company.', + '// NOLINT(build/header_guard)', + 'int64 a = (uint64) 65;', + '/* Prevent warnings about the modeline', + modeline, + '*/', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) + # LINT_KERNEL_FILE silences whitespace/tab warnings for entire file. + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('test.h', 'h', + ['// Copyright 2014 Your Company.', + '// NOLINT(build/header_guard)', + 'struct test {', + '\tint member;', + '};', + '// LINT_KERNEL_FILE', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) # Test Variable Declarations. def testVariableDeclarations(self): @@ -509,14 +585,55 @@ class CpplintTest(CpplintTestBase): self.TestLint('[](int/*unused*/) -> bool {', '') self.TestLint('[](int /*unused*/) -> bool {', '') self.TestLint('auto f = [](MyStruct* /*unused*/)->int {', '') - self.TestLint( - '[](int) -> bool {', - 'All parameters should be named in a function' - ' [readability/function] [3]') - self.TestLint( - 'auto f = [](MyStruct*)->int {', - 'All parameters should be named in a function' - ' [readability/function] [3]') + self.TestLint('[](int) -> bool {', '') + self.TestLint('auto f = [](MyStruct*)->int {', '') + + # Cast with brace initializers + self.TestLint('int64_t{4096} * 1000 * 1000', '') + self.TestLint('size_t{4096} * 1000 * 1000', '') + self.TestLint('uint_fast16_t{4096} * 1000 * 1000', '') + + # Brace initializer with templated type + self.TestMultiLineLint( + """ + template + void Function(int arg1, + int arg2) { + variable &= ~Type1{0} - 1; + }""", + '') + self.TestMultiLineLint( + """ + template + class Class { + void Function() { + variable &= ~Type{0} - 1; + } + };""", + '') + self.TestMultiLineLint( + """ + template + class Class { + void Function() { + variable &= ~Type{0} - 1; + } + };""", + '') + self.TestMultiLineLint( + """ + namespace { + template + class Class { + void Function() { + if (block) { + variable &= ~Type{0} - 1; + } + } + }; + }""", + '') # Test taking address of casts (runtime/casting) def testRuntimeCasting(self): @@ -559,6 +676,10 @@ class CpplintTest(CpplintTestBase): 'Foo::Foo(Bar r, Bel l) : r_(r_), l_(l_) { }', 'You seem to be initializing a member variable with itself.' ' [runtime/init] [4]') + self.TestLint( + 'Foo::Foo(Bar r, Bel l) : r_(CHECK_NOTNULL(r_)) { }', + 'You seem to be initializing a member variable with itself.' + ' [runtime/init] [4]') self.TestLint( 'Foo::Foo(Bar r, Bel l) : r_(r), l_(l) { }', '') @@ -568,14 +689,12 @@ class CpplintTest(CpplintTestBase): # Test for unnamed arguments in a method. def testCheckForUnnamedParams(self): - message = ('All parameters should be named in a function' - ' [readability/function] [3]') - self.TestLint('virtual void Func(int*) const;', message) - self.TestLint('virtual void Func(int*);', message) - self.TestLint('void Method(char*) {', message) - self.TestLint('void Method(char*);', message) - self.TestLint('static void operator delete[](void*) throw();', message) - self.TestLint('int Method(int);', message) + self.TestLint('virtual void Func(int*) const;', '') + self.TestLint('virtual void Func(int*);', '') + self.TestLint('void Method(char*) {', '') + self.TestLint('void Method(char*);', '') + self.TestLint('static void operator delete[](void*) throw();', '') + self.TestLint('int Method(int);', '') self.TestLint('virtual void Func(int* p);', '') self.TestLint('void operator delete(void* x) throw();', '') @@ -627,6 +746,7 @@ class CpplintTest(CpplintTestBase): self.TestLint('operator bool();', '') # Conversion operator self.TestLint('new int64(123);', '') # "new" operator on basic type self.TestLint('new int64(123);', '') # "new" operator on basic type + self.TestLint('new const int(42);', '') # "new" on const-qualified type self.TestLint('using a = bool(int arg);', '') # C++11 alias-declaration self.TestLint('x = bit_cast(y);', '') # array of array self.TestLint('void F(const char(&src)[N]);', '') # array of references @@ -838,7 +958,7 @@ class CpplintTest(CpplintTestBase): """#include "base/foobar.h" bool foobar = swap(0,1); """, - 'Add #include for swap [build/include_what_you_use] [4]') + 'Add #include for swap [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( """#include "base/foobar.h" bool foobar = transform(a.begin(), a.end(), b.start(), Foo); @@ -1199,16 +1319,13 @@ class CpplintTest(CpplintTestBase): };""", 'Zero-parameter constructors should not be marked explicit.' ' [runtime/explicit] [5]') - # Warn explicit multi-argument constructors at lowest severity + # No warning for multi-parameter constructors self.TestMultiLineLint( """ class Foo { explicit Foo(int f, int g); };""", - 'Constructors that require multiple arguments ' - 'should not be marked explicit. [runtime/explicit] [0]') - # but explicit multi-argument constructors with only one non-default - # argument are OK + '') self.TestMultiLineLint( """ class Foo { @@ -1863,18 +1980,6 @@ class CpplintTest(CpplintTestBase): 'EXPECT_TRUE(+42 >= x);', 'Consider using EXPECT_GE instead of EXPECT_TRUE(a >= b)' ' [readability/check] [2]') - self.TestLint( - 'EXPECT_TRUE_M(-42 > x);', - 'Consider using EXPECT_GT_M instead of EXPECT_TRUE_M(a > b)' - ' [readability/check] [2]') - self.TestLint( - 'EXPECT_TRUE_M(42U <= x);', - 'Consider using EXPECT_LE_M instead of EXPECT_TRUE_M(a <= b)' - ' [readability/check] [2]') - self.TestLint( - 'EXPECT_TRUE_M(42L < x);', - 'Consider using EXPECT_LT_M instead of EXPECT_TRUE_M(a < b)' - ' [readability/check] [2]') self.TestLint( 'EXPECT_FALSE(x == 42);', @@ -1896,10 +2001,6 @@ class CpplintTest(CpplintTestBase): 'ASSERT_FALSE(x <= 42);', 'Consider using ASSERT_GT instead of ASSERT_FALSE(a <= b)' ' [readability/check] [2]') - self.TestLint( - 'ASSERT_FALSE_M(x < 42);', - 'Consider using ASSERT_GE_M instead of ASSERT_FALSE_M(a < b)' - ' [readability/check] [2]') self.TestLint('CHECK(x<42);', ['Missing spaces around <' @@ -2033,6 +2134,10 @@ class CpplintTest(CpplintTestBase): self.TestLint('stream& operator>>(stream& s, Foo& f);', '') self.TestLint('stream& operator<<(stream& s, Foo& f);', '') self.TestLint('void swap(Bar& a, Bar& b);', '') + self.TestLint('ostream& LogFunc(ostream& s);', '') + self.TestLint('ostringstream& LogFunc(ostringstream& s);', '') + self.TestLint('istream& LogFunc(istream& s);', '') + self.TestLint('istringstream& LogFunc(istringstream& s);', '') # Returning a non-const reference from a function is OK. self.TestLint('int& g();', '') # Passing a const reference to a struct (using the struct keyword) is OK. @@ -2282,6 +2387,9 @@ class CpplintTest(CpplintTestBase): self.TestLint('((a+b))', '') self.TestLint('foo (foo)', 'Extra space before ( in function call' ' [whitespace/parens] [4]') + # asm volatile () may have a space, as it isn't a function call. + self.TestLint('asm volatile ("")', '') + self.TestLint('__asm__ __volatile__ ("")', '') self.TestLint('} catch (const Foo& ex) {', '') self.TestLint('case (42):', '') self.TestLint('typedef foo (*foo)(foo)', '') @@ -2319,9 +2427,20 @@ class CpplintTest(CpplintTestBase): self.TestLint('for {', '') self.TestLint('EXPECT_DEBUG_DEATH({', '') self.TestLint('std::is_convertible{}', '') + self.TestLint('blah{32}', 'Missing space before {' + ' [whitespace/braces] [5]') + self.TestLint('int8_t{3}', '') + self.TestLint('int16_t{3}', '') + self.TestLint('int32_t{3}', '') + self.TestLint('uint64_t{12345}', '') + self.TestLint('constexpr int64_t kBatchGapMicros =' + ' int64_t{7} * 24 * 3600 * 1000000; // 1 wk.', '') + self.TestLint('MoveOnly(int i1, int i2) : ip1{new int{i1}}, ' + 'ip2{new int{i2}} {}', + '') def testSemiColonAfterBraces(self): - self.TestLint('if (cond) {};', + self.TestLint('if (cond) { func(); };', 'You don\'t need a ; after a } [readability/braces] [4]') self.TestLint('void Func() {};', 'You don\'t need a ; after a } [readability/braces] [4]') @@ -2337,8 +2456,11 @@ class CpplintTest(CpplintTestBase): self.TestLint('class X : public Y {};', '') self.TestLint('class X : public MACRO() {};', '') + self.TestLint('class X : public decltype(expr) {};', '') self.TestLint('DEFINE_FACADE(PCQueue::Watcher, PCQueue) {};', '') self.TestLint('VCLASS(XfaTest, XfaContextTest) {};', '') + self.TestLint('class STUBBY_CLASS(H, E) {};', '') + self.TestLint('class STUBBY2_CLASS(H, E) {};', '') self.TestLint('TEST(TestCase, TestName) {};', 'You don\'t need a ; after a } [readability/braces] [4]') self.TestLint('TEST_F(TestCase, TestName) {};', @@ -2365,19 +2487,6 @@ class CpplintTest(CpplintTestBase): '};\n', '') - for lambda_with_default_capture in ('void f() { [=]{}; }', - 'void f() { [=](int i) {}; }', - 'void f() { [=, &x]{}; }', - 'void f() { [&]{}; }', - 'void f() { [ & ]{}; }', - 'void f() { [&, y]{}; }'): - self.TestLint(lambda_with_default_capture, - 'Default lambda captures are an unapproved C++ feature. ' - '[build/c++11] [4]') - - # "[&...]" isn't necessarily a default capture, though "[=...]" always is. - self.TestLint('void f() { [&variable]{}; }', '') - # Avoid false positives with operator[] self.TestLint('table_to_children[&*table].push_back(dependent);', '') @@ -2412,13 +2521,28 @@ class CpplintTest(CpplintTestBase): ' }\n' '};\n', '') self.TestMultiLineLint('if (true) {\n' - ' if (false){}\n' + ' if (false){ func(); }\n' '}\n', 'Missing space before { [whitespace/braces] [5]') self.TestMultiLineLint('MyClass::MyClass()\n' ' : initializer_{\n' ' Func()} {\n' '}\n', '') + self.TestLint('const pair kCL' + + ('o' * 41) + 'gStr[] = {\n', + 'Lines should be <= 80 characters long' + ' [whitespace/line_length] [2]') + self.TestMultiLineLint('const pair kCL' + + ('o' * 40) + 'ngStr[] =\n' + ' {\n' + ' {"gooooo", "oooogle"},\n' + '};\n', '') + self.TestMultiLineLint('const pair kCL' + + ('o' * 39) + 'ngStr[] =\n' + ' {\n' + ' {"gooooo", "oooogle"},\n' + '};\n', '{ should almost always be at the end of ' + 'the previous line [whitespace/braces] [4]') def testSpacingAroundElse(self): self.TestLint('}else {', 'Missing space before else' @@ -2455,6 +2579,7 @@ class CpplintTest(CpplintTestBase): 'Missing spaces around << [whitespace/operators] [3]') self.TestLint('a<>b', 'Missing spaces around >> [whitespace/operators] [3]') @@ -2481,141 +2606,6 @@ class CpplintTest(CpplintTestBase): self.TestLint('using Vector3::operator==;', '') self.TestLint('using Vector3::operator!=;', '') - def testRvalueReference(self): - space_error = 'Missing spaces around && [whitespace/operators] [3]' - rvalue_error = ('RValue references are an unapproved C++ feature.' - ' [build/c++11] [3]') - - # Places where lack of space are allowed - self.TestLint('DEFINE_BINARY_OPERATOR(&&)', '') - self.TestLint('bool operator&&(A b) {}', '') - self.TestLint('bool operator&&(A b) {', '') - self.TestLint('bool operator&&(A b);', '') - - # Assignment expressions - self.TestLint('a = b && c;', '') - self.TestLint('a = b&& c;', space_error) - self.TestLint('a = b &&c;', space_error) - self.TestLint('a = (b&& c);', space_error) - self.TestLint('a = (b &&c);', space_error) - self.TestLint('a&& b = c;', rvalue_error) - self.TestLint('a&& c = d;', rvalue_error) - self.TestLint('auto&& a = b;', rvalue_error) - self.TestLint('const a&& b = c;', rvalue_error) - self.TestLint('struct a&& b = c;', rvalue_error) - self.TestLint('decltype(a)&& b = c;', rvalue_error) - - # Cast expressions - self.TestLint('a = const_cast(c);', rvalue_error) - self.TestLint('a = const_cast(c);', rvalue_error) - self.TestLint('a = static_cast(c);', rvalue_error) - self.TestLint('a = static_cast(c);', rvalue_error) - self.TestLint('a = dynamic_cast(c);', rvalue_error) - self.TestLint('a = dynamic_cast(c);', rvalue_error) - self.TestLint('a = reinterpret_cast(c);', rvalue_error) - self.TestLint('a = reinterpret_cast(c);', rvalue_error) - self.TestLint('a = cast < b&& c;', space_error) - - # Function parameters - for indent in ['', ' ']: - for head in ['void Func', 'vector Func', 'vector\nFunc', - 'inline void Func', - 'Constructor', 'Constructor::Constructor', - 'operator=', 'operator =', 'operator = ']: - for body in [' {}', ' {', ';']: - self.TestMultiLineLint(indent + head + '(A&& b)' + body, rvalue_error) - self.TestMultiLineLint(indent + head + '(A &&b)' + body, rvalue_error) - self.TestMultiLineLint(indent + head + '(A&&... b)' + body, - rvalue_error) - self.TestMultiLineLint(indent + head + '(A&& c)' + body, - rvalue_error) - self.TestMultiLineLint(indent + head + '(A &&c)' + body, - rvalue_error) - - # Function templates - self.TestLint('std::conditional::type', rvalue_error) - self.TestLint('std::conditional::type', rvalue_error) - - # Template functions - self.TestLint('template R&& F()', rvalue_error) - self.TestLint('template R&& F() {', rvalue_error) - self.TestMultiLineLint('template \nR&& F()', rvalue_error) - self.TestMultiLineLint('template \nR&& F() {', rvalue_error) - self.TestLint('template void F(T a, R&& b)', rvalue_error) - self.TestLint('template void F(T a, R &&b)', rvalue_error) - self.TestLint('template void F(T a, R&& b) {', rvalue_error) - - # For loops - self.TestLint('for (a&& b;;)', rvalue_error) - self.TestLint('for (a&& b;;) {', rvalue_error) - self.TestLint('for (; a&& b;)', space_error) - self.TestLint('for (; a&& b;) {', space_error) - - # Constructors - self.TestLint('A(a&& b)', rvalue_error) - self.TestLint('explicit A(a&& b)', rvalue_error) - self.TestLint('A(a b) : c(d&& e)', space_error) - self.TestLint('A(a b) : c(), d(e&& f)', space_error) - - def testAllowedRvalueReference(self): - # Verify that RValue reference warnings for a line range can be silenced - error_collector = ErrorCollector(self.assert_) - cpplint.ProcessFileData('foo.cc', 'cc', - ['// Copyright 2014 Your Company.', - 'GOOGLE_ALLOW_RVALUE_REFERENCES_PUSH', - 'void F(A&& b);', - 'GOOGLE_ALLOW_RVALUE_REFERENCES_POP', - ''], - error_collector) - self.assertEquals(error_collector.Results(), '') - - # RValue references for constructors and operator= - error_collector = ErrorCollector(self.assert_) - cpplint.ProcessFileData( - 'foo.cc', 'cc', - ['// Copyright 2014 Your Company.', - 'class X {', - ' X(X&& param) = delete; // NOLINT(runtime/explicit)', - ' X(X &¶m) = default; // NOLINT(runtime/explicit)', - ' inline X(X&& param) = default; // NOLINT(runtime/explicit)', - '', - ' X& operator=(X&& param) = delete;', - ' X& operator=(X&& param) = default;', - '};', - 'A::A(A&&) = default;', - 'Outer::Inner::Inner(Inner&&) = default;', - ''], - error_collector) - self.assertEquals(error_collector.Results(), '') - - # Assume templated function parameters are forwarded, and are allowed - error_collector = ErrorCollector(self.assert_) - cpplint.ProcessFileData( - 'foo.cc', 'cc', - ['// Copyright 2014 Your Company.', - 'template ', - 'void Function1(Allowed1&& a);', - '', - 'template ', - 'void Function2(Allowed2&& a, Allowed3 &&b) {', - '}', - '', - 'template ', - 'void Function3(Ignored1 *a, Allowed4&& b) {', - '}', - '', - 'template ', - 'void Function4(Allowed5&&... a) {', - '}', - '', - 'template ', - 'void Function5(', - ' Allowed6 &&...a) {', - '}', - ''], - error_collector) - self.assertEquals(error_collector.Results(), '') - def testSpacingBeforeLastSemicolon(self): self.TestLint('call_function() ;', 'Extra space before last semicolon. If this should be an ' @@ -2649,6 +2639,11 @@ class CpplintTest(CpplintTestBase): self.TestLint('for (;;)', '') self.TestLint('for (;;) continue;', '') self.TestLint('for (;;) func();', '') + self.TestLint('if (test) {}', + 'If statement had no body and no else clause' + ' [whitespace/empty_if_body] [4]') + self.TestLint('if (test) func();', '') + self.TestLint('if (test) {} else {}', '') self.TestMultiLineLint("""while (true && false);""", 'Empty loop bodies should use {} or continue' @@ -2665,6 +2660,39 @@ class CpplintTest(CpplintTestBase): while (false);""", 'Empty loop bodies should use {} or continue' ' [whitespace/empty_loop_body] [5]') + self.TestMultiLineLint("""if (test) { + }""", + 'If statement had no body and no else clause' + ' [whitespace/empty_if_body] [4]') + self.TestMultiLineLint("""if (test, + func({})) { + }""", + 'If statement had no body and no else clause' + ' [whitespace/empty_if_body] [4]') + self.TestMultiLineLint("""if (test) + func();""", '') + self.TestLint('if (test) { hello; }', '') + self.TestLint('if (test({})) { hello; }', '') + self.TestMultiLineLint("""if (test) { + func(); + }""", '') + self.TestMultiLineLint("""if (test) { + // multiline + // comment + }""", '') + self.TestMultiLineLint("""if (test) { // comment + }""", '') + self.TestMultiLineLint("""if (test) { + } else { + }""", '') + self.TestMultiLineLint("""if (func(p1, + p2, + p3)) { + func(); + }""", '') + self.TestMultiLineLint("""if (func({}, p1)) { + func(); + }""", '') def testSpacingForRangeBasedFor(self): # Basic correctly formatted case: @@ -2693,19 +2721,49 @@ class CpplintTest(CpplintTestBase): # Static or global STL strings. def testStaticOrGlobalSTLStrings(self): + # A template for the error message for a const global/static string. error_msg = ('For a static/global string constant, use a C style ' 'string instead: "%s[]". [runtime/string] [4]') + # The error message for a non-const global/static string variable. + nonconst_error_msg = ('Static/global string variables are not permitted.' + ' [runtime/string] [4]') + self.TestLint('string foo;', - error_msg % 'char foo') + nonconst_error_msg) self.TestLint('string kFoo = "hello"; // English', - error_msg % 'char kFoo') + nonconst_error_msg) self.TestLint('static string foo;', - error_msg % 'static char foo') + nonconst_error_msg) self.TestLint('static const string foo;', error_msg % 'static const char foo') + self.TestLint('static const std::string foo;', + error_msg % 'static const char foo') self.TestLint('string Foo::bar;', - error_msg % 'char Foo::bar') + nonconst_error_msg) + + self.TestLint('std::string foo;', + nonconst_error_msg) + self.TestLint('std::string kFoo = "hello"; // English', + nonconst_error_msg) + self.TestLint('static std::string foo;', + nonconst_error_msg) + self.TestLint('static const std::string foo;', + error_msg % 'static const char foo') + self.TestLint('std::string Foo::bar;', + nonconst_error_msg) + + self.TestLint('::std::string foo;', + nonconst_error_msg) + self.TestLint('::std::string kFoo = "hello"; // English', + nonconst_error_msg) + self.TestLint('static ::std::string foo;', + nonconst_error_msg) + self.TestLint('static const ::std::string foo;', + error_msg % 'static const char foo') + self.TestLint('::std::string Foo::bar;', + nonconst_error_msg) + self.TestLint('string* pointer', '') self.TestLint('string *pointer', '') self.TestLint('string* pointer = Func();', '') @@ -2725,12 +2783,14 @@ class CpplintTest(CpplintTestBase): self.TestLint('string Foo::bar() {}', '') self.TestLint('string Foo::operator*() {}', '') # Rare case. - self.TestLint('string foo("foobar");', error_msg % 'char foo') + self.TestLint('string foo("foobar");', nonconst_error_msg) # Should not catch local or member variables. self.TestLint(' string foo', '') # Should not catch functions. self.TestLint('string EmptyString() { return ""; }', '') self.TestLint('string EmptyString () { return ""; }', '') + self.TestLint('string const& FileInfo::Pathname() const;', '') + self.TestLint('string const &FileInfo::Pathname() const;', '') self.TestLint('string VeryLongNameFunctionSometimesEndsWith(\n' ' VeryLongNameType very_long_name_variable) {}', '') self.TestLint('template<>\n' @@ -2761,21 +2821,24 @@ class CpplintTest(CpplintTestBase): 'NestedClass::MemberFunction3();', 'string TemplateClass::', 'NestedClass::MemberFunction4();', - 'string Class', + 'const string Class', '::static_member_variable1;', - 'string Class::', + 'const string Class::', 'static_member_variable2;', - 'string Class', + 'const string Class', '::static_member_variable3 = "initial value";', - 'string Class::', + 'const string Class::', 'static_member_variable4 = "initial value";', + 'string Class::', + 'static_member_variable5;', ''], error_collector) self.assertEquals(error_collector.Results(), - [error_msg % 'char Class::static_member_variable1', - error_msg % 'char Class::static_member_variable2', - error_msg % 'char Class::static_member_variable3', - error_msg % 'char Class::static_member_variable4']) + [error_msg % 'const char Class::static_member_variable1', + error_msg % 'const char Class::static_member_variable2', + error_msg % 'const char Class::static_member_variable3', + error_msg % 'const char Class::static_member_variable4', + nonconst_error_msg]) def testNoSpacesInFunctionCalls(self): self.TestLint('TellStory(1, 3);', @@ -2825,6 +2888,9 @@ class CpplintTest(CpplintTestBase): self.TestLint('// TODO(ljenkins): Fix this', '') self.TestLint('#if 1 // TEST_URLTODOCID_WHICH_HAS_THAT_WORD_IN_IT_H_', '') self.TestLint('// See also similar TODO above', '') + self.TestLint(r'EXPECT_EQ("\\", ' + r'NormalizePath("/./../foo///bar/..//x/../..", ""));', + '') def testTwoSpacesBetweenCodeAndComments(self): self.TestLint('} // namespace foo', @@ -3360,10 +3426,9 @@ class CpplintTest(CpplintTestBase): '') self.TestMultiLineLint( TrimExtraIndent(''' - static const char kNotRawString[] = "(" - ")";'''), - 'Weird number of spaces at line-start. ' - 'Are you using a 2-space indent? [whitespace/indent] [3]') + KV>'''), + '') self.TestMultiLineLint( ' static const char kSingleLineRawString[] = R"(...)";', 'Weird number of spaces at line-start. ' @@ -3723,18 +3788,18 @@ class CpplintTest(CpplintTestBase): try: cpplint._line_length = 80 self.TestLint( - '// %s' % ('H' * 77), + '// H %s' % ('H' * 75), '') self.TestLint( - '// %s' % ('H' * 78), + '// H %s' % ('H' * 76), 'Lines should be <= 80 characters long' ' [whitespace/line_length] [2]') cpplint._line_length = 120 self.TestLint( - '// %s' % ('H' * 117), + '// H %s' % ('H' * 115), '') self.TestLint( - '// %s' % ('H' * 118), + '// H %s' % ('H' * 116), 'Lines should be <= 120 characters long' ' [whitespace/line_length] [2]') finally: @@ -3873,7 +3938,8 @@ class CpplintTest(CpplintTestBase): cpplint.ProcessFileData(file_path, 'h', [], error_collector) for error in error_collector.ResultList(): matched = re.search( - 'No #ifndef header guard found, suggested CPP variable is: ([A-Z_]+)', + 'No #ifndef header guard found, suggested CPP variable is: ' + '([A-Z0-9_]+)', error) if matched is not None: return matched.group(1) @@ -4176,8 +4242,8 @@ class CpplintTest(CpplintTestBase): storage_classes = ['extern', 'register', 'static', 'typedef'] build_storage_class_error_message = ( - 'Storage class (static, extern, typedef, etc) should be first.' - ' [build/storage_class] [5]') + 'Storage-class specifier (static, extern, typedef, etc) should be ' + 'at the beginning of the declaration. [build/storage_class] [5]') # Some explicit cases. Legal in C++, deprecated in C99. self.TestLint('const int static foo = 5;', @@ -4309,6 +4375,9 @@ class Cxx11Test(CpplintTestBase): self.assertEquals(expected_error, collector.Results()) def testBlockedHeaders(self): + self.TestCxx11Feature('#include ', + 'C++ TR1 headers such as are ' + 'unapproved. [build/c++tr1] [5]') self.TestCxx11Feature('#include ', ' is an unapproved C++11 header.' ' [build/c++11] [5]') @@ -4353,6 +4422,25 @@ class Cxx11Test(CpplintTestBase): ' [build/explicit_make_pair] [4]') self.TestLint('my_make_pair', '') +class Cxx14Test(CpplintTestBase): + + def TestCxx14Feature(self, code, expected_error): + lines = code.split('\n') + collector = ErrorCollector(self.assert_) + cpplint.RemoveMultiLineComments('foo.h', lines, collector) + clean_lines = cpplint.CleansedLines(lines) + cpplint.FlagCxx14Features('foo.cc', clean_lines, 0, collector) + self.assertEquals(expected_error, collector.Results()) + + def testBlockedHeaders(self): + self.TestCxx14Feature('#include ', + ' is an unapproved C++14 header.' + ' [build/c++14] [5]') + self.TestCxx14Feature('#include ', + ' is an unapproved C++14 header.' + ' [build/c++14] [5]') + + class CleansedLinesTest(unittest.TestCase): def testInit(self): @@ -4970,6 +5058,20 @@ class CheckForFunctionLengthsTest(CpplintTestBase): 'Lint failed to find start of function body.' ' [readability/fn_size] [5]') + def testFunctionLengthCheckWithNamespace(self): + old_verbosity = cpplint._SetVerboseLevel(1) + self.TestFunctionLengthsCheck( + ('namespace {\n' + 'void CodeCoverageCL35256059() {\n' + + (' X++;\n' * 3000) + + '}\n' + '} // namespace\n'), + ('Small and focused functions are preferred: ' + 'CodeCoverageCL35256059() has 3000 non-comment lines ' + '(error triggered by exceeding 20 lines).' + ' [readability/fn_size] [5]')) + cpplint._SetVerboseLevel(old_verbosity) + def TrimExtraIndent(text_block): """Trim a uniform amount of whitespace off of each line in a string.