From 02af6281c61ddc01302801d29c35297a946998a2 Mon Sep 17 00:00:00 2001 From: "avakulenko@google.com" Date: Wed, 4 Jun 2014 18:53:25 +0000 Subject: [PATCH] Update cpplint.py to #359: 359 - Silence non-const reference warnings for derived functions. 357 - Remove the partial ban on std::move and related features. More general use of rvalue references is still banned for now. 356 - Fixed false positive for << inside macros. Also recognize implicit constructors of the form "Type(Type&& param)". 355 - Make _NestingState class public. Also adds a new method NestingState.InAsmBlock, which returns true if the top of the stack is a block containing inline ASM. 354 - Fixed false positive for multiline function templates. 353 - Fixed false positive for lambda capture. 352 - Silence RValue reference warnings that are enclosed in a GOOGLE_ALLOW_RVALUE_REFERENCES_(PUSH|POP) range. 351 - Do not warn on CR-LF lines if all input lines are uniformly CR-LF. 349 - Fixed false positive for unnamed parameters in macros. 348 - Recognize &&... as RValue references. 347 - Use alternative error message for including . 346 - Fixed false positive for function style template argument. 345 - Fixed false positive for braced constructor call inside brackets. 344 - Minor spelling and grammar fix. 343 - Fixed false positive for non-const reference check inside constructor initializer lists. 342 - Fixed cases where rvalue references are not identified correctly: - Parameter in a templated function. - Parameter for a single-arg constructor. - Return type in a templated function. 338 - Fixed false positive for deprecated cast where return type of function template is const. 337 - Fixed false positive for alias-declarations of function pointers. 336 - Improved error message for taking address of something dereferenced from a cast. 335 - Added support for C++14 digit separators (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3781.pdf). Not sure if the style guide would allow this feature or not, but cpplint must recognize these one way or another to provide accurate error messages. 333 - Fixed false positive for constructor calls for rand{} being identified as calls to rand(). 332 - Elide C++11 raw strings as an empty quoted string instead of an empty string. This allows us to differentiate blank lines inside raw strings from regular blank lines. 331 - Split up long functions in cpplint.py and cpplint_unittest.py. This is a refactoring change with zero change in functionality, the goal is to clean up new warnings. 330 - Fixed false positive for missing space check around "using operator;" 329 - Fixed false positive for indent check around multi-line raw strings. 328 - Added check missing spaces around ||. This check should have been included in the original CheckSpaces. Added check for &&, and output message for missing space or rvalue reference according to context. 327 - Fixed false positive for alias-declaration. 326 - Improved accuracy of matching parentheses and angle brackets. Previously, if cpplint was trying to match () pairs, those two characters are the only things that it looked for. This worked reasonably well for everything except <>, which is easily confused with operators. This change takes all other parentheses into account, and do not count <> characters as angle brackets if they are inside other parenthesized expressions. 325 - Fixed handling of multiple raw strings on the same line. 324 - Better enforcement that braces are used either around all branches of the condition, or none. Checks for what seem to be multiple statements in an single-line if/else body. Checks for ambigous if/if/else nesting without braces. 323 - Fixed false positive for extra space in returning lambdas. 322 - Fixed false positive for tokens with "else" prefix being treated as else keyword following a conditional block. 321 - Fixed false positive for placement new being treated as deprecated cast. 320 - Change lint so it no longer warns about use of std::function and related features (bind, placeholders) now that function/bind is no longer banned. 319 - Fixed false positive for alignof and alignas being recognized as casts. 318 - Permit std::shared_ptr, std::weak_ptr and std::enable_shared_from_this. 317 - Silence deprecated cast warning for templates using function types as the first argument 316 - Remove aligned_storage from the list of blacklisted C++11 features. 315 - Fixed false positive for casting to pointer types. 314 - Do not warn about single-arg constructors with std::initializer_list<> not marked as explicit. 313 - Remove lint errors when including . 312 - Fixed incorrect parsing of multiple block comments on the same line. 311 - Fixed nesting state parser for classes in template argument list. 310 - Fixed false positive for semicolon after brace for lambdas where there is a newline between lambda-introducer and lambda-declarator. 308 - Fixed false positive for global string pointers being treated as string values. 307 - Modify cpplint to follow updated style guide on comments in braced initializer lists. In particular, don't warn about missing spaces if the comment is aligned with the next line. 306 - Fixed false positive for brace initializer list in ternary expression. 305 - Fixed false positive for blank line at start of code block due to elided raw string contents. 304 - Add a cpplint.py warning for default captures in lambda expressions. 303 - Recognize unordered_map and unordered_set. 302 - Fixed false positive for trailing semicolons when lambda-capture spans multiple lines. 301 - Fixed false positive for trailing semicolon following lambdas. 300 - Fix raw string handling when the next raw string begins on the same line that the previous raw string ends. 299 - Fix false C-style cast detection due to trailing "override". 298 - Fix false positive for requiring an argument name in a GMock declaration. 297 - Fixed false positives for blank line warnings near 'extern "C"' blocks. R=erg@google.com Review URL: https://codereview.appspot.com/108730043 --- cpplint/cpplint.py | 2064 +++++++++++++++++++++++++---------- cpplint/cpplint_unittest.py | 927 ++++++++++++++-- 2 files changed, 2301 insertions(+), 690 deletions(-) diff --git a/cpplint/cpplint.py b/cpplint/cpplint.py index 76d0735..36ad300 100755 --- a/cpplint/cpplint.py +++ b/cpplint/cpplint.py @@ -114,7 +114,7 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] ignored. Examples: - Assuing that src/.git exists, the header guard CPP variables for + Assuming that src/.git exists, the header guard CPP variables for src/chrome/browser/ui/browser.h are: No flag => CHROME_BROWSER_UI_BROWSER_H_ @@ -141,6 +141,7 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] # here! cpplint_unittest.py should tell you if you forget to do this. _ERROR_CATEGORIES = [ 'build/class', + 'build/c++11', 'build/deprecated', 'build/endif_comment', 'build/explicit_make_pair', @@ -203,7 +204,7 @@ _ERROR_CATEGORIES = [ 'whitespace/todo' ] -# The default state of the category filter. This is overrided by the --filter= +# The default state of the category filter. This is overridden by the --filter= # flag. By default all errors are on, so only add here categories that should be # off by default (i.e., categories that must be enabled by the --filter= flags). # All entries here should start with a '-' or '+', as in the --filter= flag. @@ -213,7 +214,6 @@ _DEFAULT_FILTERS = ['-build/include_alpha'] # 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. - # C++ headers _CPP_HEADERS = frozenset([ # Legacy @@ -350,6 +350,7 @@ _CPP_HEADERS = frozenset([ 'cwctype', ]) + # 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. @@ -500,6 +501,7 @@ def IsErrorSuppressedByNolint(category, linenum): return (linenum in _error_suppressions.get(category, set()) or linenum in _error_suppressions.get(None, set())) + def Match(pattern, s): """Matches the string with the pattern, caching the compiled regexp.""" # The regexp compilation caching is inlined in both Match and Search for @@ -954,6 +956,7 @@ def _ShouldPrintError(category, confidence, linenum): # the verbosity level isn't high enough, or the filters filter it out. if IsErrorSuppressedByNolint(category, linenum): return False + if confidence < _cpplint_state.verbose_level: return False @@ -1011,11 +1014,9 @@ def Error(filename, linenum, category, confidence, message): # Matches standard C++ escape sequences per 2.13.2.3 of the C++ standard. _RE_PATTERN_CLEANSE_LINE_ESCAPES = re.compile( r'\\([abfnrtv?"\\\']|\d+|x[0-9a-fA-F]+)') -# Matches strings. Escape codes should already be removed by ESCAPES. -_RE_PATTERN_CLEANSE_LINE_DOUBLE_QUOTES = re.compile(r'"[^"]*"') -# Matches characters. Escape codes should already be removed by ESCAPES. -_RE_PATTERN_CLEANSE_LINE_SINGLE_QUOTES = re.compile(r"'.'") -# Matches multi-line C++ comments. +# Match a single C style comment on the same line. +_RE_PATTERN_C_COMMENTS = r'/\*(?:[^*]|\*(?!/))*\*/' +# Matches multi-line C style comments. # This RE is a little bit more complicated than one might expect, because we # have to take care of space removals tools so we can handle comments inside # statements better. @@ -1024,10 +1025,10 @@ _RE_PATTERN_CLEANSE_LINE_SINGLE_QUOTES = re.compile(r"'.'") # if this doesn't work we try on left side but only if there's a non-character # on the right. _RE_PATTERN_CLEANSE_LINE_C_COMMENTS = re.compile( - r"""(\s*/\*.*\*/\s*$| - /\*.*\*/\s+| - \s+/\*.*\*/(?=\W)| - /\*.*\*/)""", re.VERBOSE) + r'(\s*' + _RE_PATTERN_C_COMMENTS + r'\s*$|' + + _RE_PATTERN_C_COMMENTS + r'\s+|' + + r'\s+' + _RE_PATTERN_C_COMMENTS + r'(?=\W)|' + + _RE_PATTERN_C_COMMENTS + r')') def IsCppString(line): @@ -1082,9 +1083,12 @@ def CleanseRawStrings(raw_lines): delimiter = None else: # Haven't found the end yet, append a blank line. - line = '' + line = '""' - else: + # Look for beginning of a raw string, and replace them with + # empty strings. This is done in a loop to handle multiple raw + # strings on the same line. + 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) @@ -1100,6 +1104,8 @@ def CleanseRawStrings(raw_lines): else: # Start of a multi-line raw string line = matched.group(1) + '""' + else: + break lines_without_raw_strings.append(line) @@ -1205,38 +1211,138 @@ class CleansedLines(object): Returns: The line with collapsed strings. """ - if not _RE_PATTERN_INCLUDE.match(elided): - # Remove escaped characters first to make quote/single quote collapsing - # basic. Things that look like escaped characters shouldn't occur - # outside of strings and chars. - elided = _RE_PATTERN_CLEANSE_LINE_ESCAPES.sub('', elided) - elided = _RE_PATTERN_CLEANSE_LINE_SINGLE_QUOTES.sub("''", elided) - elided = _RE_PATTERN_CLEANSE_LINE_DOUBLE_QUOTES.sub('""', elided) - return elided + if _RE_PATTERN_INCLUDE.match(elided): + return elided + + # Remove escaped characters first to make quote/single quote collapsing + # basic. Things that look like escaped characters shouldn't occur + # outside of strings and chars. + elided = _RE_PATTERN_CLEANSE_LINE_ESCAPES.sub('', elided) + + # Replace quoted strings and digit separators. Both single quotes + # and double quotes are processed in the same loop, otherwise + # nested quotes wouldn't work. + collapsed = '' + while True: + # Find the first quote character + match = Match(r'^([^\'"]*)([\'"])(.*)$', elided) + if not match: + collapsed += elided + break + head, quote, tail = match.groups() + + if quote == '"': + # Collapse double quoted strings + second_quote = tail.find('"') + if second_quote >= 0: + collapsed += head + '""' + elided = tail[second_quote + 1:] + else: + # Unmatched double quote, don't bother processing the rest + # of the line since this is probably a multiline string. + collapsed += elided + break + else: + # Found single quote, check nearby text to eliminate digit separators. + # + # There is no special handling for floating point here, because + # the integer/fractional/exponent parts would all be parsed + # correctly as long as there are digits on both sides of the + # separator. So we are fine as long as we don't see something + # like "0.'3" (gcc 4.9.0 will not allow this literal). + if Search(r'\b(?:0[bBxX]?|[1-9])[0-9a-fA-F]*$', head): + match_literal = Match(r'^((?:\'?[0-9a-zA-Z_])*)(.*)$', "'" + tail) + collapsed += head + match_literal.group(1).replace("'", '') + elided = match_literal.group(2) + else: + second_quote = tail.find('\'') + if second_quote >= 0: + collapsed += head + "''" + elided = tail[second_quote + 1:] + else: + # Unmatched single quote + collapsed += elided + break + + return collapsed -def FindEndOfExpressionInLine(line, startpos, depth, startchar, endchar): - """Find the position just after the matching endchar. +def FindEndOfExpressionInLine(line, startpos, stack): + """Find the position just after the end of current parenthesized expression. Args: line: a CleansedLines line. startpos: start searching at this position. - depth: nesting level at startpos. - startchar: expression opening character. - endchar: expression closing character. + stack: nesting stack at startpos. Returns: - On finding matching endchar: (index just after matching endchar, 0) - Otherwise: (-1, new depth at end of this line) + On finding matching end: (index just after matching end, None) + On finding an unclosed expression: (-1, None) + Otherwise: (-1, new stack at end of this line) """ for i in xrange(startpos, len(line)): - if line[i] == startchar: - depth += 1 - elif line[i] == endchar: - depth -= 1 - if depth == 0: - return (i + 1, 0) - return (-1, depth) + char = line[i] + if char in '([{': + # Found start of parenthesized expression, push to expression stack + stack.append(char) + elif char == '<': + # Found potential start of template argument list + if i > 0 and line[i - 1] == '<': + # Left shift operator + if stack and stack[-1] == '<': + stack.pop() + if not stack: + return (-1, None) + elif i > 0 and Search(r'\boperator\s*$', line[0:i]): + # operator<, don't add to stack + continue + else: + # Tentative start of template argument list + stack.append('<') + elif char in ')]}': + # Found end of parenthesized expression. + # + # If we are currently expecting a matching '>', the pending '<' + # must have been an operator. Remove them from expression stack. + while stack and stack[-1] == '<': + stack.pop() + if not stack: + return (-1, None) + if ((stack[-1] == '(' and char == ')') or + (stack[-1] == '[' and char == ']') or + (stack[-1] == '{' and char == '}')): + stack.pop() + if not stack: + return (i + 1, None) + else: + # Mismatched parentheses + return (-1, None) + elif char == '>': + # Found potential end of template argument list. + + # Ignore "->" and operator functions + if (i > 0 and + (line[i - 1] == '-' or Search(r'\boperator\s*$', line[0:i - 1]))): + continue + + # Pop the stack if there is a matching '<'. Otherwise, ignore + # this '>' since it must be an operator. + if stack: + if stack[-1] == '<': + stack.pop() + if not stack: + return (i + 1, None) + elif char == ';': + # Found something that look like end of statements. If we are currently + # expecting a '>', the matching '<' must have been an operator, since + # template argument list should not contain statements. + while stack and stack[-1] == '<': + stack.pop() + if not stack: + return (-1, None) + + # Did not find end of expression or unbalanced parentheses on this line + return (-1, stack) def CloseExpression(clean_lines, linenum, pos): @@ -1245,6 +1351,11 @@ def CloseExpression(clean_lines, linenum, pos): If lines[linenum][pos] points to a '(' or '{' or '[' or '<', finds the linenum/pos that correspond to the closing of the expression. + TODO(unknown): cpplint spends a fair bit of time matching parentheses. + Ideally we would want to index all opening and closing parentheses once + and have CloseExpression be just a simple lookup, but due to preprocessor + tricks, this is not so easy. + Args: clean_lines: A CleansedLines instance containing the file. linenum: The number of the line to check. @@ -1258,35 +1369,28 @@ def CloseExpression(clean_lines, linenum, pos): """ line = clean_lines.elided[linenum] - startchar = line[pos] - if startchar not in '({[<': + if (line[pos] not in '({[<') or Match(r'<[<=]', line[pos:]): return (line, clean_lines.NumLines(), -1) - if startchar == '(': endchar = ')' - if startchar == '[': endchar = ']' - if startchar == '{': endchar = '}' - if startchar == '<': endchar = '>' # Check first line - (end_pos, num_open) = FindEndOfExpressionInLine( - line, pos, 0, startchar, endchar) + (end_pos, stack) = FindEndOfExpressionInLine(line, pos, []) if end_pos > -1: return (line, linenum, end_pos) # Continue scanning forward - while linenum < clean_lines.NumLines() - 1: + while stack and linenum < clean_lines.NumLines() - 1: linenum += 1 line = clean_lines.elided[linenum] - (end_pos, num_open) = FindEndOfExpressionInLine( - line, 0, num_open, startchar, endchar) + (end_pos, stack) = FindEndOfExpressionInLine(line, 0, stack) if end_pos > -1: return (line, linenum, end_pos) - # Did not find endchar before end of file, give up + # Did not find end of expression before end of file, give up return (line, clean_lines.NumLines(), -1) -def FindStartOfExpressionInLine(line, endpos, depth, startchar, endchar): - """Find position at the matching startchar. +def FindStartOfExpressionInLine(line, endpos, stack): + """Find position at the matching start of current expression. This is almost the reverse of FindEndOfExpressionInLine, but note that the input position and returned position differs by 1. @@ -1294,22 +1398,72 @@ def FindStartOfExpressionInLine(line, endpos, depth, startchar, endchar): Args: line: a CleansedLines line. endpos: start searching at this position. - depth: nesting level at endpos. - startchar: expression opening character. - endchar: expression closing character. + stack: nesting stack at endpos. Returns: - On finding matching startchar: (index at matching startchar, 0) - Otherwise: (-1, new depth at beginning of this line) + On finding matching start: (index at matching start, None) + On finding an unclosed expression: (-1, None) + Otherwise: (-1, new stack at beginning of this line) """ - for i in xrange(endpos, -1, -1): - if line[i] == endchar: - depth += 1 - elif line[i] == startchar: - depth -= 1 - if depth == 0: - return (i, 0) - return (-1, depth) + i = endpos + while i >= 0: + char = line[i] + if char in ')]}': + # Found end of expression, push to expression stack + stack.append(char) + elif char == '>': + # Found potential end of template argument list. + # + # Ignore it if it's a "->" or ">=" or "operator>" + if (i > 0 and + (line[i - 1] == '-' or + Match(r'\s>=\s', line[i - 1:]) or + Search(r'\boperator\s*$', line[0:i]))): + i -= 1 + else: + stack.append('>') + elif char == '<': + # Found potential start of template argument list + if i > 0 and line[i - 1] == '<': + # Left shift operator + i -= 1 + else: + # If there is a matching '>', we can pop the expression stack. + # Otherwise, ignore this '<' since it must be an operator. + if stack and stack[-1] == '>': + stack.pop() + if not stack: + return (i, None) + elif char in '([{': + # Found start of expression. + # + # If there are any unmatched '>' on the stack, they must be + # operators. Remove those. + while stack and stack[-1] == '>': + stack.pop() + if not stack: + return (-1, None) + if ((char == '(' and stack[-1] == ')') or + (char == '[' and stack[-1] == ']') or + (char == '{' and stack[-1] == '}')): + stack.pop() + if not stack: + return (i, None) + else: + # Mismatched parentheses + return (-1, None) + elif char == ';': + # Found something that look like end of statements. If we are currently + # expecting a '<', the matching '>' must have been an operator, since + # template argument list should not contain statements. + while stack and stack[-1] == '>': + stack.pop() + if not stack: + return (-1, None) + + i -= 1 + + return (-1, stack) def ReverseCloseExpression(clean_lines, linenum, pos): @@ -1330,30 +1484,23 @@ def ReverseCloseExpression(clean_lines, linenum, pos): return is the 'cleansed' line at linenum. """ line = clean_lines.elided[linenum] - endchar = line[pos] - if endchar not in ')}]>': + if line[pos] not in ')}]>': return (line, 0, -1) - if endchar == ')': startchar = '(' - if endchar == ']': startchar = '[' - if endchar == '}': startchar = '{' - if endchar == '>': startchar = '<' # Check last line - (start_pos, num_open) = FindStartOfExpressionInLine( - line, pos, 0, startchar, endchar) + (start_pos, stack) = FindStartOfExpressionInLine(line, pos, []) if start_pos > -1: return (line, linenum, start_pos) # Continue scanning backward - while linenum > 0: + while stack and linenum > 0: linenum -= 1 line = clean_lines.elided[linenum] - (start_pos, num_open) = FindStartOfExpressionInLine( - line, len(line) - 1, num_open, startchar, endchar) + (start_pos, stack) = FindStartOfExpressionInLine(line, len(line) - 1, stack) if start_pos > -1: return (line, linenum, start_pos) - # Did not find startchar before beginning of file, give up + # Did not find start of expression before beginning of file, give up return (line, 0, -1) @@ -1370,6 +1517,22 @@ def CheckForCopyright(filename, lines, error): 'You should have a line: "Copyright [year] "') +def GetIndentLevel(line): + """Return the number of leading spaces in line. + + Args: + line: A string to check. + + Returns: + An integer count of leading spaces, possibly zero. + """ + indent = Match(r'^( *)\S', line) + if indent: + return len(indent.group(1)) + else: + return 0 + + def GetHeaderGuardCPPVariable(filename): """Returns the CPP variable that should be used as a header guard. @@ -1550,19 +1713,33 @@ def CheckForMultilineCommentsAndStrings(filename, clean_lines, linenum, error): 'Use C++11 raw strings or concatenation instead.') -threading_list = ( - ('asctime(', 'asctime_r('), - ('ctime(', 'ctime_r('), - ('getgrgid(', 'getgrgid_r('), - ('getgrnam(', 'getgrnam_r('), - ('getlogin(', 'getlogin_r('), - ('getpwnam(', 'getpwnam_r('), - ('getpwuid(', 'getpwuid_r('), - ('gmtime(', 'gmtime_r('), - ('localtime(', 'localtime_r('), - ('rand(', 'rand_r('), - ('strtok(', 'strtok_r('), - ('ttyname(', 'ttyname_r('), +# (non-threadsafe name, thread-safe alternative, validation pattern) +# +# The validation pattern is used to eliminate false positives such as: +# _rand(); // false positive due to substring match. +# ->rand(); // some member function rand(). +# ACMRandom rand(seed); // some variable named rand. +# ISAACRandom rand(); // another variable named rand. +# +# Basically we require the return value of these functions to be used +# in some expression context on the same line by matching on some +# operator before the function name. This eliminates constructors and +# member function calls. +_UNSAFE_FUNC_PREFIX = r'(?:[-+*/=%^&|(<]\s*|>\s+)' +_THREADING_LIST = ( + ('asctime(', 'asctime_r(', _UNSAFE_FUNC_PREFIX + r'asctime\([^)]+\)'), + ('ctime(', 'ctime_r(', _UNSAFE_FUNC_PREFIX + r'ctime\([^)]+\)'), + ('getgrgid(', 'getgrgid_r(', _UNSAFE_FUNC_PREFIX + r'getgrgid\([^)]+\)'), + ('getgrnam(', 'getgrnam_r(', _UNSAFE_FUNC_PREFIX + r'getgrnam\([^)]+\)'), + ('getlogin(', 'getlogin_r(', _UNSAFE_FUNC_PREFIX + r'getlogin\(\)'), + ('getpwnam(', 'getpwnam_r(', _UNSAFE_FUNC_PREFIX + r'getpwnam\([^)]+\)'), + ('getpwuid(', 'getpwuid_r(', _UNSAFE_FUNC_PREFIX + r'getpwuid\([^)]+\)'), + ('gmtime(', 'gmtime_r(', _UNSAFE_FUNC_PREFIX + r'gmtime\([^)]+\)'), + ('localtime(', 'localtime_r(', _UNSAFE_FUNC_PREFIX + r'localtime\([^)]+\)'), + ('rand(', 'rand_r(', _UNSAFE_FUNC_PREFIX + r'rand\(\)'), + ('strtok(', 'strtok_r(', + _UNSAFE_FUNC_PREFIX + r'strtok\([^)]+\)'), + ('ttyname(', 'ttyname_r(', _UNSAFE_FUNC_PREFIX + r'ttyname\([^)]+\)'), ) @@ -1582,14 +1759,13 @@ def CheckPosixThreading(filename, clean_lines, linenum, error): error: The function to call with any errors found. """ line = clean_lines.elided[linenum] - for single_thread_function, multithread_safe_function in threading_list: - ix = line.find(single_thread_function) - # Comparisons made explicit for clarity -- pylint: disable=g-explicit-bool-comparison - if ix >= 0 and (ix == 0 or (not line[ix - 1].isalnum() and - line[ix - 1] not in ('_', '.', '>'))): + for single_thread_func, multithread_safe_func, pattern in _THREADING_LIST: + # Additional pattern matching check to confirm that this is the + # function we are looking for + if Search(pattern, line): error(filename, linenum, 'runtime/threadsafe_fn', 2, - 'Consider using ' + multithread_safe_function + - '...) instead of ' + single_thread_function + + 'Consider using ' + multithread_safe_func + + '...) instead of ' + single_thread_func + '...) for improved thread safety.') @@ -1611,7 +1787,6 @@ def CheckVlogArguments(filename, clean_lines, linenum, error): 'VLOG() should be used with numeric verbosity level. ' 'Use LOG() if you want symbolic severity levels.') - # Matches invalid increment: *count++, which moves pointer instead of # incrementing a value. _RE_PATTERN_INVALID_INCREMENT = re.compile( @@ -1676,6 +1851,24 @@ class _BlockInfo(object): """ pass + def IsBlockInfo(self): + """Returns true if this block is a _BlockInfo. + + This is convenient for verifying that an object is an instance of + a _BlockInfo, but not an instance of any of the derived classes. + + Returns: + True for this class, False for derived classes. + """ + return self.__class__ == _BlockInfo + + +class _ExternCInfo(_BlockInfo): + """Stores information about an 'extern "C"' block.""" + + def __init__(self): + _BlockInfo.__init__(self, True) + class _ClassInfo(_BlockInfo): """Stores information about a class.""" @@ -1694,11 +1887,7 @@ class _ClassInfo(_BlockInfo): # Remember initial indentation level for this class. Using raw_lines here # instead of elided to account for leading comments. - initial_indent = Match(r'^( *)\S', clean_lines.raw_lines[linenum]) - if initial_indent: - self.class_indent = len(initial_indent.group(1)) - else: - self.class_indent = 0 + self.class_indent = GetIndentLevel(clean_lines.raw_lines[linenum]) # Try to find the end of the class. This will be confused by things like: # class A { @@ -1783,8 +1972,15 @@ class _NamespaceInfo(_BlockInfo): else: # Anonymous namespace if not Match(r'};*\s*(//|/\*).*\bnamespace[\*/\.\\\s]*$', line): - error(filename, linenum, 'readability/namespace', 5, - 'Namespace should be terminated with "// namespace"') + # 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): + error(filename, linenum, 'readability/namespace', 5, + 'Anonymous namespace should be terminated with "// namespace"' + ' or "// anonymous namespace"') + else: + error(filename, linenum, 'readability/namespace', 5, + 'Anonymous namespace should be terminated with "// namespace"') class _PreprocessorInfo(object): @@ -1801,7 +1997,7 @@ class _PreprocessorInfo(object): self.seen_else = False -class _NestingState(object): +class NestingState(object): """Holds states related to parsing braces.""" def __init__(self): @@ -1813,6 +2009,17 @@ class _NestingState(object): # - _BlockInfo: some other type of block. self.stack = [] + # Top of the previous stack before each Update(). + # + # Because the nesting_stack is updated at the end of each line, we + # had to do some convoluted checks to find out what is the current + # scope at the beginning of the line. This check is simplified by + # saving the previous top of nesting stack. + # + # We could save the full stack, but we only need the top. Copying + # the full nesting stack would slow down cpplint by ~10%. + self.previous_stack_top = [] + # Stack of _PreprocessorInfo objects. self.pp_stack = [] @@ -1833,6 +2040,82 @@ class _NestingState(object): """ return self.stack and isinstance(self.stack[-1], _NamespaceInfo) + def InExternC(self): + """Check if we are currently one level inside an 'extern "C"' block. + + Returns: + True if top of the stack is an extern block, False otherwise. + """ + return self.stack and isinstance(self.stack[-1], _ExternCInfo) + + def InClassDeclaration(self): + """Check if we are currently one level inside a class or struct declaration. + + Returns: + True if top of the stack is a class/struct, False otherwise. + """ + return self.stack and isinstance(self.stack[-1], _ClassInfo) + + def InAsmBlock(self): + """Check if we are currently one level inside an inline ASM block. + + Returns: + True if the top of the stack is a block containing inline ASM. + """ + return self.stack and self.stack[-1].inline_asm != _NO_ASM + + def InTemplateArgumentList(self, clean_lines, linenum, pos): + """Check if current position is inside template argument list. + + Args: + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + pos: position just after the suspected template argument. + Returns: + True if (linenum, pos) is inside template arguments. + """ + while linenum < clean_lines.NumLines(): + # Find the earliest character that might indicate a template argument + line = clean_lines.elided[linenum] + match = Match(r'^[^{};=\[\]\.<>]*(.)', line[pos:]) + if not match: + linenum += 1 + pos = 0 + continue + token = match.group(1) + pos += len(match.group(0)) + + # These things do not look like template argument list: + # class Suspect { + # class Suspect x; } + if token in ('{', '}', ';'): return False + + # These things look like template argument list: + # template + # template + # template + # template + if token in ('>', '=', '[', ']', '.'): return True + + # Check if token is an unmatched '<'. + # If not, move on to the next character. + if token != '<': + pos += 1 + if pos >= len(line): + linenum += 1 + pos = 0 + continue + + # We can't be sure if we just find a single '<', and need to + # find the matching '>'. + (_, end_line, end_pos) = CloseExpression(clean_lines, linenum, pos - 1) + if end_pos < 0: + # Not sure if template argument list or syntax error in file + return False + linenum = end_line + pos = end_pos + return False + def UpdatePreprocessor(self, line): """Update preprocessor stack. @@ -1889,6 +2172,7 @@ class _NestingState(object): # TODO(unknown): unexpected #endif, issue warning? pass + # TODO(unknown): Update() is too long, but we will refactor later. def Update(self, filename, clean_lines, linenum, error): """Update nesting state with current line. @@ -1900,7 +2184,17 @@ class _NestingState(object): """ line = clean_lines.elided[linenum] - # Update pp_stack first + # Remember top of the previous nesting stack. + # + # The stack is always pushed/popped and not modified in place, so + # we can just do a shallow copy instead of copy.deepcopy. Using + # deepcopy would slow down cpplint by ~28%. + if self.stack: + self.previous_stack_top = self.stack[-1] + else: + self.previous_stack_top = None + + # Update pp_stack self.UpdatePreprocessor(line) # Count parentheses. This is to avoid adding struct arguments to @@ -1951,32 +2245,27 @@ class _NestingState(object): # such as in: # class LOCKABLE API Object { # }; - # - # Templates with class arguments may confuse the parser, for example: - # template , - # class Vector = vector > - # class HeapQueue { - # - # Because this parser has no nesting state about templates, by the - # time it saw "class Comparator", it may think that it's a new class. - # Nested templates have a similar problem: - # template < - # typename ExportedType, - # typename TupleType, - # template class ImplTemplate> - # - # To avoid these cases, we ignore classes that are followed by '=' or '>' class_decl_match = Match( - r'\s*(template\s*<[\w\s<>,:]*>\s*)?' - r'(class|struct)\s+([A-Z_]+\s+)*(\w+(?:::\w+)*)' - r'(([^=>]|<[^<>]*>|<[^<>]*<[^<>]*>\s*>)*)$', line) + r'^(\s*(?:template\s*<[\w\s<>,:]*>\s*)?' + r'(class|struct)\s+(?:[A-Z_]+\s+)*(\w+(?:::\w+)*))' + r'(.*)$', line) if (class_decl_match and (not self.stack or self.stack[-1].open_parentheses == 0)): - self.stack.append(_ClassInfo( - class_decl_match.group(4), class_decl_match.group(2), - clean_lines, linenum)) - line = class_decl_match.group(5) + # We do not want to accept classes that are actually template arguments: + # template , + # template class Ignore3> + # void Function() {}; + # + # To avoid template argument cases, we scan forward and look for + # an unmatched '>'. If we see one, assume we are inside a + # template argument list. + end_declaration = len(class_decl_match.group(1)) + if not self.InTemplateArgumentList(clean_lines, linenum, end_declaration): + self.stack.append(_ClassInfo( + class_decl_match.group(3), class_decl_match.group(2), + clean_lines, linenum)) + line = class_decl_match.group(4) # If we have not yet seen the opening brace for the innermost block, # run checks here. @@ -2023,10 +2312,13 @@ class _NestingState(object): # stack otherwise. if not self.SeenOpenBrace(): self.stack[-1].seen_open_brace = True + elif Match(r'^extern\s*"[^"]*"\s*\{', line): + self.stack.append(_ExternCInfo()) else: self.stack.append(_BlockInfo(True)) if _MATCH_ASM.match(line): self.stack[-1].inline_asm = _BLOCK_ASM + elif token == ';' or token == ')': # If we haven't seen an opening brace yet, but we already saw # a semicolon, this is probably a forward declaration. Pop @@ -2102,7 +2394,7 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, 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 + nesting_state: A NestingState instance which maintains information about the current stack of nested blocks being parsed. error: A callable to which errors are reported, which takes 4 arguments: filename, line number, error level, and message @@ -2180,21 +2472,23 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, line) if (args and args.group(1) != 'void' and + not Search(r'\bstd::initializer_list\b', args.group(1)) and not Match(r'(const\s+)?%s(\s+const)?\s*(?:<\w+>\s*)?&' % re.escape(base_classname), args.group(1).strip())): error(filename, linenum, 'runtime/explicit', 5, 'Single-argument constructors should be marked explicit.') -def CheckSpacingForFunctionCall(filename, line, linenum, error): +def CheckSpacingForFunctionCall(filename, clean_lines, linenum, error): """Checks for the correctness of various spacing around function calls. Args: filename: The name of the current file. - line: The text of the line to check. + 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] # Since function calls often occur inside if/for/while/switch # expressions - which have their own, more liberal conventions - we @@ -2237,10 +2531,16 @@ def CheckSpacingForFunctionCall(filename, line, linenum, error): error(filename, linenum, 'whitespace/parens', 2, 'Extra space after (') if (Search(r'\w\s+\(', fncall) and - not Search(r'#\s*define|typedef', fncall) and + not Search(r'#\s*define|typedef|using\s+\w+\s*=', fncall) and not Search(r'\w\s+\((\w+::)*\*\w+\)\(', fncall)): - error(filename, linenum, 'whitespace/parens', 4, - 'Extra space before ( in function call') + # TODO(unknown): Space after an operator function seem to be a common + # error, silence those for now by restricting them to highest verbosity. + if Search(r'\boperator_*\b', line): + error(filename, linenum, 'whitespace/parens', 0, + 'Extra space before ( in function call') + else: + error(filename, linenum, 'whitespace/parens', 4, + 'Extra space before ( in function call') # If the ) is followed only by a newline or a { + newline, assume it's # part of a control statement (if/while/etc), and don't complain if Search(r'[^)]\s+\)\s*[^{\s]', fncall): @@ -2294,8 +2594,6 @@ def CheckForFunctionLengths(filename, clean_lines, linenum, """ lines = clean_lines.lines line = lines[linenum] - raw = clean_lines.raw_lines - raw_line = raw[linenum] joined_line = '' starting_func = False @@ -2342,34 +2640,58 @@ def CheckForFunctionLengths(filename, clean_lines, linenum, _RE_PATTERN_TODO = re.compile(r'^//(\s*)TODO(\(.+?\))?:?(\s|$)?') -def CheckComment(comment, filename, linenum, error): - """Checks for common mistakes in TODO comments. +def CheckComment(line, filename, linenum, next_line_start, error): + """Checks for common mistakes in comments. Args: - comment: The text of the comment from the line in question. + line: The line in question. filename: The name of the current file. linenum: The number of the line to check. + next_line_start: The first non-whitespace column of the next line. error: The function to call with any errors found. """ - match = _RE_PATTERN_TODO.match(comment) - if match: - # One whitespace is correct; zero whitespace is handled elsewhere. - leading_whitespace = match.group(1) - if len(leading_whitespace) > 1: - error(filename, linenum, 'whitespace/todo', 2, - 'Too many spaces before TODO') + 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 + # Allow one space for new scopes, two spaces otherwise: + if (not (Match(r'^.*{ *//', line) and next_line_start == commentpos) and + ((commentpos >= 1 and + line[commentpos-1] not in string.whitespace) or + (commentpos >= 2 and + line[commentpos-2] not in string.whitespace))): + error(filename, linenum, 'whitespace/comments', 2, + 'At least two spaces is best between code and comments') - username = match.group(2) - if not username: - error(filename, linenum, 'readability/todo', 2, - 'Missing username in TODO; it should look like ' - '"// TODO(my_username): Stuff."') + # Checks for common mistakes in TODO comments. + comment = line[commentpos:] + match = _RE_PATTERN_TODO.match(comment) + if match: + # One whitespace is correct; zero whitespace is handled elsewhere. + leading_whitespace = match.group(1) + if len(leading_whitespace) > 1: + error(filename, linenum, 'whitespace/todo', 2, + 'Too many spaces before TODO') - middle_whitespace = match.group(3) - # Comparisons made explicit for correctness -- pylint: disable=g-explicit-bool-comparison - if middle_whitespace != ' ' and middle_whitespace != '': - error(filename, linenum, 'whitespace/todo', 2, - 'TODO(my_username) should be followed by a space') + username = match.group(2) + if not username: + error(filename, linenum, 'readability/todo', 2, + 'Missing username in TODO; it should look like ' + '"// TODO(my_username): Stuff."') + + middle_whitespace = match.group(3) + # Comparisons made explicit for correctness -- pylint: disable=g-explicit-bool-comparison + if middle_whitespace != ' ' and middle_whitespace != '': + error(filename, linenum, 'whitespace/todo', 2, + 'TODO(my_username) should be followed by a space') + + # If the comment contains an alphanumeric character, there + # should be a space somewhere between it and the //. + if Match(r'//[^ ]*\w', comment): + error(filename, linenum, 'whitespace/comments', 4, + 'Should have a space between // and comment') def CheckAccess(filename, clean_lines, linenum, nesting_state, error): """Checks for improper use of DISALLOW* macros. @@ -2378,7 +2700,7 @@ def CheckAccess(filename, clean_lines, linenum, nesting_state, error): 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 + 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. """ @@ -2402,132 +2724,6 @@ def CheckAccess(filename, clean_lines, linenum, nesting_state, error): pass -def FindNextMatchingAngleBracket(clean_lines, linenum, init_suffix): - """Find the corresponding > to close a template. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: Current line number. - init_suffix: Remainder of the current line after the initial <. - - Returns: - True if a matching bracket exists. - """ - line = init_suffix - nesting_stack = ['<'] - while True: - # Find the next operator that can tell us whether < is used as an - # opening bracket or as a less-than operator. We only want to - # warn on the latter case. - # - # We could also check all other operators and terminate the search - # early, e.g. if we got something like this "a(),;\[\]]*([<>(),;\[\]])(.*)$', line) - if match: - # Found an operator, update nesting stack - operator = match.group(1) - line = match.group(2) - - if nesting_stack[-1] == '<': - # Expecting closing angle bracket - if operator in ('<', '(', '['): - nesting_stack.append(operator) - elif operator == '>': - nesting_stack.pop() - if not nesting_stack: - # Found matching angle bracket - return True - elif operator == ',': - # Got a comma after a bracket, this is most likely a template - # argument. We have not seen a closing angle bracket yet, but - # it's probably a few lines later if we look for it, so just - # return early here. - return True - else: - # Got some other operator. - return False - - else: - # Expecting closing parenthesis or closing bracket - if operator in ('<', '(', '['): - nesting_stack.append(operator) - elif operator in (')', ']'): - # We don't bother checking for matching () or []. If we got - # something like (] or [), it would have been a syntax error. - nesting_stack.pop() - - else: - # Scan the next line - linenum += 1 - if linenum >= len(clean_lines.elided): - break - line = clean_lines.elided[linenum] - - # Exhausted all remaining lines and still no matching angle bracket. - # Most likely the input was incomplete, otherwise we should have - # seen a semicolon and returned early. - return True - - -def FindPreviousMatchingAngleBracket(clean_lines, linenum, init_prefix): - """Find the corresponding < that started a template. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: Current line number. - init_prefix: Part of the current line before the initial >. - - Returns: - True if a matching bracket exists. - """ - line = init_prefix - nesting_stack = ['>'] - while True: - # Find the previous operator - match = Search(r'^(.*)([<>(),;\[\]])[^<>(),;\[\]]*$', line) - if match: - # Found an operator, update nesting stack - operator = match.group(2) - line = match.group(1) - - if nesting_stack[-1] == '>': - # Expecting opening angle bracket - if operator in ('>', ')', ']'): - nesting_stack.append(operator) - elif operator == '<': - nesting_stack.pop() - if not nesting_stack: - # Found matching angle bracket - return True - elif operator == ',': - # Got a comma before a bracket, this is most likely a - # template argument. The opening angle bracket is probably - # there if we look for it, so just return early here. - return True - else: - # Got some other operator. - return False - - else: - # Expecting opening parenthesis or opening bracket - if operator in ('>', ')', ']'): - nesting_stack.append(operator) - elif operator in ('(', '['): - nesting_stack.pop() - - else: - # Scan the previous line - linenum -= 1 - if linenum < 0: - break - line = clean_lines.elided[linenum] - - # Exhausted all earlier lines and still no matching angle bracket. - return False - - def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): """Checks for the correctness of various spacing issues in the code. @@ -2541,7 +2737,7 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): 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 + 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. """ @@ -2564,7 +2760,12 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): # } # # A warning about missing end of namespace comments will be issued instead. - if IsBlankLine(line) and not nesting_state.InNamespaceBody(): + # + # Also skip blank line checks for 'extern "C"' blocks, which are formatted + # like namespaces. + if (IsBlankLine(line) and + not nesting_state.InNamespaceBody() and + not nesting_state.InExternC()): elided = clean_lines.elided prev_line = elided[linenum - 1] prevbrace = prev_line.rfind('{') @@ -2627,48 +2828,53 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): error(filename, linenum, 'whitespace/blank_line', 3, 'Do not leave a blank line after "%s:"' % matched.group(1)) - # Next, we complain if there's a comment too near the text - 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 - # Allow one space for new scopes, two spaces otherwise: - if (not Match(r'^\s*{ //', line) and - ((commentpos >= 1 and - line[commentpos-1] not in string.whitespace) or - (commentpos >= 2 and - line[commentpos-2] not in string.whitespace))): - error(filename, linenum, 'whitespace/comments', 2, - 'At least two spaces is best between code and comments') - # There should always be a space between the // and the comment - commentend = commentpos + 2 - if commentend < len(line) and not line[commentend] == ' ': - # but some lines are exceptions -- e.g. if they're big - # comment delimiters like: - # //---------------------------------------------------------- - # or are an empty C++ style Doxygen comment, like: - # /// - # or C++ style Doxygen comments placed after the variable: - # ///< Header comment - # //!< Header comment - # or they begin with multiple slashes followed by a space: - # //////// Header comment - match = (Search(r'[=/-]{4,}\s*$', line[commentend:]) or - Search(r'^/$', line[commentend:]) or - Search(r'^!< ', line[commentend:]) or - Search(r'^/< ', line[commentend:]) or - Search(r'^/+ ', line[commentend:])) - if not match: - error(filename, linenum, 'whitespace/comments', 4, - 'Should have a space between // and comment') - CheckComment(line[commentpos:], filename, linenum, error) + # Next, check comments + next_line_start = 0 + if linenum + 1 < clean_lines.NumLines(): + next_line = raw[linenum + 1] + next_line_start = len(next_line) - len(next_line.lstrip()) + CheckComment(line, filename, linenum, next_line_start, error) - line = clean_lines.elided[linenum] # get rid of comments and strings + # get rid of comments and strings + line = clean_lines.elided[linenum] - # Don't try to do spacing checks for operator methods - line = re.sub(r'operator(==|!=|<|<<|<=|>=|>>|>)\(', 'operator\(', line) + # You shouldn't have spaces before your brackets, except maybe after + # 'delete []' or 'return []() {};' + if Search(r'\w\s+\[', line) and not Search(r'(?:delete|return)\s+\[', line): + error(filename, linenum, 'whitespace/braces', 5, + 'Extra space before [') + + # In range-based for, we wanted spaces before and after the colon, but + # not around "::" tokens that might appear. + if (Search(r'for *\(.*[^:]:[^: ]', line) or + Search(r'for *\(.*[^: ]:[^:]', line)): + error(filename, linenum, 'whitespace/forcolon', 2, + 'Missing space around colon in range-based for loop') + + +def CheckOperatorSpacing(filename, clean_lines, linenum, error): + """Checks for horizontal spacing around operators. + + 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] + + # Don't try to do spacing checks for operator methods. Do this by + # replacing the troublesome characters with something else, + # preserving column position for all other characters. + # + # The replacement is done repeatedly to avoid false positives from + # operators that call operators. + while True: + match = Match(r'^(.*\boperator\b)(\S+)(\s*\(.*)$', line) + if match: + line = match.group(1) + ('_' * len(match.group(2))) + match.group(3) + else: + break # We allow no-spaces around = within an if: "if ( (a=Foo()) == 0 )". # Otherwise not. Note we only check for non-spaces on *both* sides; @@ -2686,42 +2892,51 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): # # Check <= and >= first to avoid false positives with < and >, then # check non-include lines for spacing around < and >. - match = Search(r'[^<>=!\s](==|!=|<=|>=)[^<>=!\s]', line) + # + # If the operator is followed by a comma, assume it's be used in a + # macro context and don't do any checks. This avoids false + # positives. + # + # Note that && is not included here. Those are checked separately + # in CheckRValueReference + match = Search(r'[^<>=!\s](==|!=|<=|>=|\|\|)[^<>=!\s,;\)]', line) if match: error(filename, linenum, 'whitespace/operators', 3, 'Missing spaces around %s' % match.group(1)) - # We allow no-spaces around << when used like this: 10<<20, but - # not otherwise (particularly, not when used as streams) - # Also ignore using ns::operator<<; - match = Search(r'(operator|\S)(?:L|UL|ULL|l|ul|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, - 'Missing spaces around <<') elif not Match(r'#.*include', line): - # Avoid false positives on -> - reduced_line = line.replace('->', '') - # Look for < that is not surrounded by spaces. This is only # triggered if both sides are missing spaces, even though # technically should should flag if at least one side is missing a # space. This is done to avoid some false positives with shifts. - match = Search(r'[^\s<]<([^\s=<].*)', reduced_line) - if (match and - not FindNextMatchingAngleBracket(clean_lines, linenum, match.group(1))): - error(filename, linenum, 'whitespace/operators', 3, - 'Missing spaces around <') + match = Match(r'^(.*[^\s<])<[^\s=<,]', line) + if match: + (_, _, end_pos) = CloseExpression( + clean_lines, linenum, len(match.group(1))) + if end_pos <= -1: + error(filename, linenum, 'whitespace/operators', 3, + 'Missing spaces around <') # Look for > that is not surrounded by spaces. Similar to the # above, we only trigger if both sides are missing spaces to avoid # false positives with shifts. - match = Search(r'^(.*[^\s>])>[^\s=>]', reduced_line) - if (match and - not FindPreviousMatchingAngleBracket(clean_lines, linenum, - match.group(1))): - error(filename, linenum, 'whitespace/operators', 3, - 'Missing spaces around >') + match = Match(r'^(.*[^-\s>])>[^\s=>,]', line) + if match: + (_, _, start_pos) = ReverseCloseExpression( + clean_lines, linenum, len(match.group(1))) + if start_pos <= -1: + error(filename, linenum, 'whitespace/operators', 3, + 'Missing spaces around >') + + # We allow no-spaces around << when used like this: 10<<20, but + # not otherwise (particularly, not when used as streams) + # 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) + if (match and match.group(1) != '(' 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, + 'Missing spaces around <<') # We allow no-spaces around >> for almost anything. This is because # C++11 allows ">>" to close nested templates, which accounts for @@ -2746,7 +2961,19 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): error(filename, linenum, 'whitespace/operators', 4, 'Extra space for operator %s' % match.group(1)) - # A pet peeve of mine: no spaces after an if, while, switch, or for + +def CheckParenthesisSpacing(filename, clean_lines, linenum, error): + """Checks for horizontal spacing around parentheses. + + 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] + + # No spaces after an if, while, switch, or for match = Search(r' (if\(|for\(|while\(|switch\()', line) if match: error(filename, linenum, 'whitespace/parens', 5, @@ -2772,6 +2999,19 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): 'Should have zero or one spaces inside ( and ) in %s' % match.group(1)) + +def CheckCommaSpacing(filename, clean_lines, linenum, error): + """Checks for horizontal spacing near commas and semicolons. + + 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. + """ + raw = clean_lines.lines_without_raw_strings + line = clean_lines.elided[linenum] + # You should always have a space after a comma (either as fn arg or operator) # # This does not apply when the non-space character following the @@ -2794,8 +3034,17 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): error(filename, linenum, 'whitespace/semicolon', 3, 'Missing space after ;') - # Next we will look for issues with function calls. - CheckSpacingForFunctionCall(filename, line, linenum, error) + +def CheckBracesSpacing(filename, clean_lines, linenum, 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. + 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 @@ -2812,10 +3061,12 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): # LastArgument(..., type{}); # LOG(INFO) << type{} << " ..."; # map_of_type[{...}] = ...; + # ternary = expr ? new type{} : nullptr; + # OuterTemplate{}> # # We check for the character following the closing brace, and # silence the warning if it's one of those listed above, i.e. - # "{.;,)<]". + # "{.;,)<>]:". # # To account for nested initializer list, we allow any number of # closing braces up to "{;,)<". We can't simply silence the @@ -2837,7 +3088,7 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, 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): + if not Match(r'^[\s}]*[{.;,)<>\]:]', trailing_text): error(filename, linenum, 'whitespace/braces', 5, 'Missing space before {') @@ -2846,12 +3097,6 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): error(filename, linenum, 'whitespace/braces', 5, 'Missing space before else') - # You shouldn't have spaces before your brackets, except maybe after - # 'delete []' or 'new char * []'. - if Search(r'\w\s+\[', line) and not Search(r'delete\s+\[', line): - error(filename, linenum, 'whitespace/braces', 5, - 'Extra space before [') - # You shouldn't have a space before a semicolon at the end of the line. # There's a special case for "for" since the style guide allows space before # the semicolon there. @@ -2868,12 +3113,299 @@ def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): 'Extra space before last semicolon. If this should be an empty ' 'statement, use {} instead.') - # In range-based for, we wanted spaces before and after the colon, but - # not around "::" tokens that might appear. - if (Search('for *\(.*[^:]:[^: ]', line) or - Search('for *\(.*[^: ]:[^:]', line)): - error(filename, linenum, 'whitespace/forcolon', 2, - 'Missing space around colon in range-based for loop') + +def IsDecltype(clean_lines, linenum, column): + """Check if the token ending on (linenum, column) is decltype(). + + 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 decltype() expression, False otherwise. + """ + (text, _, start_col) = ReverseCloseExpression(clean_lines, linenum, column) + if start_col < 0: + return False + if Search(r'\bdecltype\s*$', text[0:start_col]): + return True + 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(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: + 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 simple type 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 ['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'\bexplicit$', 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(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 IsRValueAllowed(clean_lines, linenum): + """Check if RValue reference is allowed within some range of lines. + + Args: + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + Returns: + True if line is within the region where RValue references are allowed. + """ + 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') + return False + + +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. + and_pos = len(match.group(1)) + if IsRValueType(clean_lines, nesting_state, linenum, and_pos): + if not IsRValueAllowed(clean_lines, linenum): + 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): @@ -2981,7 +3513,7 @@ def CheckBraces(filename, clean_lines, linenum, error): '{ should almost always be at the end of the previous line') # An else clause should be on the same line as the preceding closing brace. - if Match(r'\s*else\s*', line): + if Match(r'\s*else\b\s*(?:if\b|\{|$)', line): prevline = GetPreviousNonBlankLine(clean_lines, linenum)[0] if Match(r'\s*}\s*$', prevline): error(filename, linenum, 'whitespace/newline', 4, @@ -2989,19 +3521,20 @@ def CheckBraces(filename, clean_lines, linenum, error): # If braces come on one side of an else, they should be on both. # However, we have to worry about "else if" that spans multiple lines! - if Search(r'}\s*else[^{]*$', line) or Match(r'[^}]*else\s*{', line): - if Search(r'}\s*else if([^{]*)$', line): # could be multi-line if - # find the ( after the if - pos = line.find('else if') - pos = line.find('(', pos) - if pos > 0: - (endline, _, endpos) = CloseExpression(clean_lines, linenum, pos) - if endline[endpos:].find('{') == -1: # must be brace after if - error(filename, linenum, 'readability/braces', 5, - 'If an else has a brace on one side, it should have it on both') - else: # common case: else not followed by a multi-line if - error(filename, linenum, 'readability/braces', 5, - 'If an else has a brace on one side, it should have it on both') + if Search(r'else if\s*\(', line): # could be multi-line if + brace_on_left = bool(Search(r'}\s*else if\s*\(', line)) + # find the ( after the if + pos = line.find('else if') + pos = line.find('(', pos) + if pos > 0: + (endline, _, endpos) = CloseExpression(clean_lines, linenum, pos) + brace_on_right = endline[endpos:].find('{') != -1 + if brace_on_left != brace_on_right: # must be brace after if + error(filename, linenum, 'readability/braces', 5, + 'If an else has a brace on one side, it should have it on both') + elif Search(r'}\s*else[^{]*$', line) or Match(r'[^}]*else\s*{', line): + error(filename, linenum, 'readability/braces', 5, + 'If an else has a brace on one side, it should have it on both') # Likewise, an else should never have the else clause on the same line if Search(r'\belse [^\s{]', line) and not Search(r'\belse if\b', line): @@ -3013,6 +3546,70 @@ def CheckBraces(filename, clean_lines, linenum, error): error(filename, linenum, 'whitespace/newline', 4, 'do/while clauses should not be on a single line') + # Check single-line if/else bodies. The style guide says 'curly braces are not + # required for single-line statements'. We additionally allow multi-line, + # single statements, but we reject anything with more than one semicolon in + # it. This means that the first semicolon after the if should be at the end of + # its line, and the line after that should have an indent level equal to or + # lower than the if. We also check for ambiguous if/else nesting without + # braces. + if_else_match = Search(r'\b(if\s*\(|else\b)', line) + if if_else_match and not Match(r'\s*#', line): + if_indent = GetIndentLevel(line) + endline, endlinenum, endpos = line, linenum, if_else_match.end() + if_match = Search(r'\bif\s*\(', line) + if if_match: + # This could be a multiline if condition, so find the end first. + pos = if_match.end() - 1 + (endline, endlinenum, endpos) = CloseExpression(clean_lines, linenum, pos) + # Check for an opening brace, either directly after the if or on the next + # line. If found, this isn't a single-statement conditional. + if (not Match(r'\s*{', endline[endpos:]) + and not (Match(r'\s*$', endline[endpos:]) + and endlinenum < (len(clean_lines.elided) - 1) + and Match(r'\s*{', clean_lines.elided[endlinenum + 1]))): + while (endlinenum < len(clean_lines.elided) + and ';' not in clean_lines.elided[endlinenum][endpos:]): + endlinenum += 1 + endpos = 0 + if endlinenum < len(clean_lines.elided): + endline = clean_lines.elided[endlinenum] + # We allow a mix of whitespace and closing braces (e.g. for one-liner + # methods) and a single \ after the semicolon (for macros) + endpos = endline.find(';') + if not Match(r';[\s}]*(\\?)$', endline[endpos:]): + # Semicolon isn't the last character, there's something trailing + error(filename, linenum, 'readability/braces', 4, + 'If/else bodies with multiple statements require braces') + elif endlinenum < len(clean_lines.elided) - 1: + # Make sure the next line is dedented + next_line = clean_lines.elided[endlinenum + 1] + next_indent = GetIndentLevel(next_line) + # With ambiguous nested if statements, this will error out on the + # if that *doesn't* match the else, regardless of whether it's the + # inner one or outer one. + if (if_match and Match(r'\s*else\b', next_line) + and next_indent != if_indent): + error(filename, linenum, 'readability/braces', 4, + 'Else clause should be indented at the same level as if. ' + 'Ambiguous nested if/else chains require braces.') + elif next_indent > if_indent: + error(filename, linenum, 'readability/braces', 4, + 'If/else bodies with multiple statements require braces') + + +def CheckTrailingSemicolon(filename, clean_lines, linenum, error): + """Looks for redundant trailing semicolon. + + 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] + # Block bodies should not be followed by a semicolon. Due to C++11 # brace initialization, there are more places where semicolons are # required than not, so we use a whitelist approach to check these @@ -3081,20 +3678,27 @@ def CheckBraces(filename, clean_lines, linenum, error): # would result in compile errors. # # In addition to macros, we also don't want to warn on compound - # literals. + # literals and lambdas. 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) + func = Match(r'^(.*\])\s*$', line_prefix) if ((macro and macro.group(1) not in ( 'TEST', 'TEST_F', 'MATCHER', 'MATCHER_P', 'TYPED_TEST', 'EXCLUSIVE_LOCKS_REQUIRED', 'SHARED_LOCKS_REQUIRED', 'LOCKS_EXCLUDED', 'INTERFACE_DEF')) or + (func and not Search(r'\boperator\s*\[\s*\]', func.group(1))) or Search(r'\s+=\s*$', line_prefix)): match = None + if (match and + opening_parenthesis[1] > 1 and + Search(r'\]\s*$', clean_lines.elided[opening_parenthesis[1] - 1])): + # Multi-line lambda-expression + match = None else: # Try matching cases 2-3. @@ -3163,6 +3767,29 @@ def CheckEmptyBlockBody(filename, clean_lines, linenum, error): 'Empty loop bodies should use {} or continue') +def FindCheckMacro(line): + """Find a replaceable CHECK-like macro. + + Args: + line: line to search on. + Returns: + (macro name, start position), or (None, -1) if no replaceable + macro is found. + """ + for macro in _CHECK_MACROS: + i = line.find(macro) + if i >= 0: + # Find opening parenthesis. Do a regular expression match here + # to make sure that we are matching the expected CHECK macro, as + # opposed to some other macro that happens to contain the CHECK + # substring. + matched = Match(r'^(.*\b' + macro + r'\s*)\(', line) + if not matched: + continue + return (macro, len(matched.group(1))) + return (None, -1) + + def CheckCheck(filename, clean_lines, linenum, error): """Checks the use of CHECK and EXPECT macros. @@ -3175,24 +3802,8 @@ def CheckCheck(filename, clean_lines, linenum, error): # Decide the set of replacement macros that should be suggested lines = clean_lines.elided - check_macro = None - start_pos = -1 - for macro in _CHECK_MACROS: - i = lines[linenum].find(macro) - if i >= 0: - check_macro = macro - - # Find opening parenthesis. Do a regular expression match here - # to make sure that we are matching the expected CHECK macro, as - # opposed to some other macro that happens to contain the CHECK - # substring. - matched = Match(r'^(.*\b' + check_macro + r'\s*)\(', lines[linenum]) - if not matched: - continue - start_pos = len(matched.group(1)) - break - if not check_macro or start_pos < 0: - # Don't waste time here if line doesn't contain 'CHECK' or 'EXPECT' + (check_macro, start_pos) = FindCheckMacro(lines[linenum]) + if not check_macro: return # Find end of the boolean expression by matching parentheses @@ -3222,7 +3833,7 @@ def CheckCheck(filename, clean_lines, linenum, error): if token == '(': # Parenthesized operand expression = matched.group(2) - (end, _) = FindEndOfExpressionInLine(expression, 0, 1, '(', ')') + (end, _) = FindEndOfExpressionInLine(expression, 0, ['(']) if end < 0: return # Unmatched parenthesis lhs += '(' + expression[0:end] @@ -3357,7 +3968,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, clean_lines: A CleansedLines instance containing the file. linenum: The number of the line to check. file_extension: The extension (without the dot) of the filename. - nesting_state: A _NestingState instance which maintains information about + 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. """ @@ -3384,6 +3995,8 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # if(match($0, " <<")) complain = 0; # if(match(prev, " +for \\(")) complain = 0; # if(prevodd && match(prevprev, " +for \\(")) complain = 0; + scope_or_label_pattern = r'\s*\w+\s*:\s*\\?$' + classinfo = nesting_state.InnermostClass() initial_spaces = 0 cleansed_line = clean_lines.elided[linenum] while initial_spaces < len(line) and line[initial_spaces] == ' ': @@ -3391,9 +4004,12 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, 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 + # 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(r'\s*\w+\s*:\s*$', cleansed_line)): + 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?') @@ -3441,9 +4057,16 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # Some more style checks CheckBraces(filename, clean_lines, linenum, error) + CheckTrailingSemicolon(filename, clean_lines, linenum, error) CheckEmptyBlockBody(filename, clean_lines, linenum, error) CheckAccess(filename, clean_lines, linenum, nesting_state, error) CheckSpacing(filename, clean_lines, linenum, nesting_state, error) + CheckOperatorSpacing(filename, clean_lines, linenum, error) + CheckParenthesisSpacing(filename, clean_lines, linenum, error) + CheckCommaSpacing(filename, clean_lines, linenum, error) + CheckBracesSpacing(filename, clean_lines, linenum, 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() @@ -3580,7 +4203,6 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): error: The function to call with any errors found. """ fileinfo = FileInfo(filename) - line = clean_lines.lines[linenum] # "include" should use the new style "foo/bar.h" instead of just "bar.h" @@ -3633,8 +4255,13 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): if Match(r'(f|ind|io|i|o|parse|pf|stdio|str|)?stream$', include): # Many unit tests use cout, so we exempt them. if not _IsTestFilename(filename): - error(filename, linenum, 'readability/streams', 3, - 'Streams are highly discouraged.') + # Suggest a different header for ostream + if include == 'ostream': + error(filename, linenum, 'readability/streams', 3, + 'For logging, include "base/logging.h" instead of .') + else: + error(filename, linenum, 'readability/streams', 3, + 'Streams are highly discouraged.') def _GetTextInside(text, start_pattern): @@ -3657,7 +4284,7 @@ def _GetTextInside(text, start_pattern): The extracted text. None if either the opening string or ending punctuation could not be found. """ - # TODO(sugawarayu): Audit cpplint.py to see what places could be profitably + # TODO(unknown): Audit cpplint.py to see what places could be profitably # rewritten to use _GetTextInside (and use inferior regexp matching today). # Give opening punctuations to get the matching close-punctuations. @@ -3732,7 +4359,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, linenum: The number of the line to check. file_extension: The extension (without the dot) of the filename. include_state: An _IncludeState instance in which the headers are inserted. - nesting_state: A _NestingState instance which maintains information about + 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. """ @@ -3754,118 +4381,11 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, # Make Windows paths like Unix. fullname = os.path.abspath(filename).replace('\\', '/') - - # TODO(unknown): figure out if they're using default arguments in fn proto. - - # Check to see if they're using an conversion function cast. - # I just try to capture the most common basic types, though there are more. - # Parameterless conversion functions, such as bool(), are allowed as they are - # probably a member operator declaration or default constructor. - match = Search( - r'(\bnew\s+)?\b' # Grab 'new' operator, if it's there - r'(int|float|double|bool|char|int32|uint32|int64|uint64)' - r'(\([^)].*)', line) - if match: - matched_new = match.group(1) - matched_type = match.group(2) - matched_funcptr = match.group(3) - - # gMock methods are defined using some variant of MOCK_METHODx(name, type) - # where type may be float(), int(string), etc. Without context they are - # virtually indistinguishable from int(x) casts. Likewise, gMock's - # MockCallback takes a template parameter of the form return_type(arg_type), - # which looks much like the cast we're trying to detect. - # - # std::function<> wrapper has a similar problem. - # - # Return types for function pointers also look like casts if they - # don't have an extra space. - if (matched_new is None and # If new operator, then this isn't a cast - not (Match(r'^\s*MOCK_(CONST_)?METHOD\d+(_T)?\(', line) or - Search(r'\bMockCallback<.*>', line) or - Search(r'\bstd::function<.*>', line)) and - not (matched_funcptr and - Match(r'\((?:[^() ]+::\s*\*\s*)?[^() ]+\)\s*\(', - matched_funcptr))): - # Try a bit harder to catch gmock lines: the only place where - # something looks like an old-style cast is where we declare the - # return type of the mocked method, and the only time when we - # are missing context is if MOCK_METHOD was split across - # multiple lines. The missing MOCK_METHOD is usually one or two - # lines back, so scan back one or two lines. - # - # It's not possible for gmock macros to appear in the first 2 - # lines, since the class head + section name takes up 2 lines. - if (linenum < 2 or - not (Match(r'^\s*MOCK_(?:CONST_)?METHOD\d+(?:_T)?\((?:\S+,)?\s*$', - clean_lines.elided[linenum - 1]) or - Match(r'^\s*MOCK_(?:CONST_)?METHOD\d+(?:_T)?\(\s*$', - clean_lines.elided[linenum - 2]))): - error(filename, linenum, 'readability/casting', 4, - 'Using deprecated casting style. ' - 'Use static_cast<%s>(...) instead' % - matched_type) - - CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], - 'static_cast', - r'\((int|float|double|bool|char|u?int(16|32|64))\)', error) - - # This doesn't catch all cases. Consider (const char * const)"hello". - # - # (char *) "foo" should always be a const_cast (reinterpret_cast won't - # compile). - if CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], - 'const_cast', r'\((char\s?\*+\s?)\)\s*"', error): - pass - else: - # Check pointer casts for other than string constants - CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], - 'reinterpret_cast', r'\((\w+\s?\*+\s?)\)', error) - - # In addition, we look for people taking the address of a cast. This - # is dangerous -- casts can assign to temporaries, so the pointer doesn't - # point where you think. - match = Search( - r'(?:&\(([^)]+)\)[\w(])|' - r'(?:&(static|dynamic|down|reinterpret)_cast\b)', line) - if match and match.group(1) != '*': - error(filename, linenum, 'runtime/casting', 4, - ('Are you taking an address of a cast? ' - 'This is dangerous: could be a temp var. ' - 'Take the address before doing the cast, rather than after')) - - # Create an extended_line, which is the concatenation of the current and - # next lines, for more effective checking of code that may span more than one - # line. - if linenum + 1 < clean_lines.NumLines(): - extended_line = line + clean_lines.elided[linenum + 1] - else: - extended_line = line - - # 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. - match = Match( - r'((?:|static +)(?:|const +))string +([a-zA-Z0-9_:]+)\b(.*)', - line) - # Make sure it's not a function. - # Function template specialization looks like: "string foo(...". - # Class template definitions look like: "string Foo::Method(...". - # - # Also ignore things that look like operators. These are matched separately - # because operator names cross non-word boundaries. If we change the pattern - # above, we would decrease the accuracy of matching identifiers. - if (match 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))) - - if Search(r'\b([A-Za-z0-9_]*_)\(\1\)', line): - error(filename, linenum, 'runtime/init', 4, - 'You seem to be initializing a member variable with itself.') + + # 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) + CheckPrintf(filename, clean_lines, linenum, error) if file_extension == 'h': # TODO(unknown): check that 1-arg constructors are explicit. @@ -3887,23 +4407,6 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, error(filename, linenum, 'runtime/int', 4, 'Use int16/int64/etc, rather than the C type %s' % match.group(1)) - # When snprintf is used, the second argument shouldn't be a literal. - match = Search(r'snprintf\s*\(([^,]*),\s*([0-9]*)\s*,', line) - if match and match.group(2) != '0': - # If 2nd arg is zero, snprintf is used to calculate size. - error(filename, linenum, 'runtime/printf', 3, - 'If you can, use sizeof(%s) instead of %s as the 2nd arg ' - 'to snprintf.' % (match.group(1), match.group(2))) - - # Check if some verboten C functions are being used. - if Search(r'\bsprintf\b', line): - error(filename, linenum, 'runtime/printf', 5, - 'Never use sprintf. Use snprintf instead.') - match = Search(r'\b(strcpy|strcat)\b', line) - if match: - error(filename, linenum, 'runtime/printf', 4, - 'Almost always, snprintf is better than %s' % match.group(1)) - # Check if some verboten operator overloading is going on # TODO(unknown): catch out-of-line unary operator&: # class X {}; @@ -3923,7 +4426,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, # Check for potential format string bugs like printf(foo). # We constrain the pattern not to pick things like DocidForPrintf(foo). # Not perfect but it can catch printf(foo.c_str()) and printf(foo->c_str()) - # TODO(sugawarayu): Catch the following case. Need to change the calling + # TODO(unknown): Catch the following case. Need to change the calling # convention of the whole function to process multiple line to handle it. # printf( # boy_this_is_a_really_long_variable_that_cannot_fit_on_the_prev_line); @@ -4019,6 +4522,144 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, 'http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces' ' for more information.') + +def CheckGlobalStatic(filename, clean_lines, linenum, error): + """Check for unsafe global or static objects. + + 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] + + # 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. + match = Match( + r'((?:|static +)(?:|const +))string +([a-zA-Z0-9_:]+)\b(.*)', + line) + # Remove false positives: + # - String pointers (as opposed to values). + # string *pointer + # const string *pointer + # string const *pointer + # string *const pointer + # + # - Functions and template specializations. + # string Function(... + # string Class::Method(... + # + # - Operators. These are matched separately because operator names + # cross non-word boundaries, and trying to match both operators + # and functions at the same time would decrease accuracy of + # 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'\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))) + + if Search(r'\b([A-Za-z0-9_]*_)\(\1\)', line): + error(filename, linenum, 'runtime/init', 4, + 'You seem to be initializing a member variable with itself.') + + +def CheckPrintf(filename, clean_lines, linenum, error): + """Check for printf related issues. + + 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] + + # When snprintf is used, the second argument shouldn't be a literal. + match = Search(r'snprintf\s*\(([^,]*),\s*([0-9]*)\s*,', line) + if match and match.group(2) != '0': + # If 2nd arg is zero, snprintf is used to calculate size. + error(filename, linenum, 'runtime/printf', 3, + 'If you can, use sizeof(%s) instead of %s as the 2nd arg ' + 'to snprintf.' % (match.group(1), match.group(2))) + + # Check if some verboten C functions are being used. + if Search(r'\bsprintf\b', line): + error(filename, linenum, 'runtime/printf', 5, + 'Never use sprintf. Use snprintf instead.') + match = Search(r'\b(strcpy|strcat)\b', line) + if match: + error(filename, linenum, 'runtime/printf', 4, + 'Almost always, snprintf is better than %s' % match.group(1)) + + +def IsDerivedFunction(clean_lines, linenum): + """Check if current line contains an inherited function. + + Args: + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + Returns: + True if current line contains a function with "override" + virt-specifier. + """ + # Look for leftmost opening parenthesis on current line + opening_paren = clean_lines.elided[linenum].find('(') + if opening_paren < 0: return False + + # Look for "override" after the matching closing parenthesis + line, _, closing_paren = CloseExpression(clean_lines, linenum, opening_paren) + return closing_paren >= 0 and Search(r'\boverride\b', line[closing_paren:]) + + +def IsInitializerList(clean_lines, linenum): + """Check if current line is inside constructor initializer list. + + Args: + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + Returns: + True if current line appears to be inside constructor initializer + list, False otherwise. + """ + for i in xrange(linenum, 1, -1): + line = clean_lines.elided[i] + if i == linenum: + remove_function_body = Match(r'^(.*)\{\s*$', line) + if remove_function_body: + line = remove_function_body.group(1) + + if Search(r'\s:\s*\w+[({]', line): + # A lone colon tend to indicate the start of a constructor + # initializer list. It could also be a ternary operator, which + # also tend to appear in constructor initializer lists as + # opposed to parameter lists. + return True + if Search(r'\}\s*,\s*$', line): + # A closing brace followed by a comma is probably the end of a + # brace-initialized member in constructor initializer list. + return True + if Search(r'[{};]\s*$', line): + # Found one of the following: + # - A closing brace or semicolon, probably the end of the previous + # function. + # - An opening brace, probably the start of current class or namespace. + # + # Current line is probably not inside an initializer list since + # we saw one of those things without seeing the starting colon. + return False + + # Got to the beginning of the file without seeing the start of + # constructor initializer list. + return False + + def CheckForNonConstReference(filename, clean_lines, linenum, nesting_state, error): """Check for non-const references. @@ -4030,7 +4671,7 @@ def CheckForNonConstReference(filename, clean_lines, linenum, 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 + 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. """ @@ -4039,6 +4680,12 @@ def CheckForNonConstReference(filename, clean_lines, linenum, if '&' not in line: return + # If a function is inherited, current function doesn't have much of + # a choice, so any non-const references should not be blamed on + # derived function. + if IsDerivedFunction(clean_lines, linenum): + return + # Long type names may be broken across multiple lines, usually in one # of these forms: # LongType @@ -4087,19 +4734,21 @@ def CheckForNonConstReference(filename, clean_lines, linenum, # inside declarators: reference parameter # We will exclude the first two cases by checking that we are not inside a # function body, including one that was just introduced by a trailing '{'. - # TODO(unknwon): Doesn't account for preprocessor directives. # TODO(unknown): Doesn't account for 'catch(Exception& e)' [rare]. - check_params = False - if not nesting_state.stack: - check_params = True # top level - elif (isinstance(nesting_state.stack[-1], _ClassInfo) or - isinstance(nesting_state.stack[-1], _NamespaceInfo)): - check_params = True # within class or namespace - elif Match(r'.*{\s*$', line): - if (len(nesting_state.stack) == 1 or - isinstance(nesting_state.stack[-2], _ClassInfo) or - isinstance(nesting_state.stack[-2], _NamespaceInfo)): - check_params = True # just opened global/class/namespace block + if (nesting_state.previous_stack_top and + not (isinstance(nesting_state.previous_stack_top, _ClassInfo) or + isinstance(nesting_state.previous_stack_top, _NamespaceInfo))): + # Not at toplevel, not within a class, and not within a namespace + return + + # Avoid preprocessors + if Search(r'\\\s*$', line): + return + + # Avoid constructor initializer lists + if IsInitializerList(clean_lines, linenum): + return + # We allow non-const references in a few standard places, like functions # called "swap()" or iostream operators like "<<" or ">>". Do not check # those function parameters. @@ -4111,7 +4760,7 @@ def CheckForNonConstReference(filename, clean_lines, linenum, r'static_assert|COMPILE_ASSERT' r')\s*\(') if Search(whitelisted_functions, line): - check_params = False + return elif not Search(r'\S+\([^)]*$', line): # Don't see a whitelisted function on this line. Actually we # didn't see any function name on this line, so this is likely a @@ -4119,17 +4768,122 @@ def CheckForNonConstReference(filename, clean_lines, linenum, for i in xrange(2): if (linenum > i and Search(whitelisted_functions, clean_lines.elided[linenum - i - 1])): - check_params = False - break + return - if check_params: - 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): - error(filename, linenum, 'runtime/references', 2, - 'Is this a non-const reference? ' - 'If so, make const or use a pointer: ' + - ReplaceAll(' *<', '<', parameter)) + 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): + error(filename, linenum, 'runtime/references', 2, + 'Is this a non-const reference? ' + 'If so, make const or use a pointer: ' + + ReplaceAll(' *<', '<', parameter)) + + +def CheckCasts(filename, clean_lines, linenum, error): + """Various cast related checks. + + 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] + + # Check to see if they're using an conversion function cast. + # I just try to capture the most common basic types, though there are more. + # 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'(int|float|double|bool|char|int32|uint32|int64|uint64)' + r'(\([^)].*)', line) + expecting_function = ExpectingFunctionArgs(clean_lines, linenum) + if match and not expecting_function: + matched_type = match.group(2) + + # matched_new_or_template is used to silence two false positives: + # - New operators + # - Template arguments with function types + # + # For template arguments, we match on types immediately following + # an opening bracket without any spaces. This is a fast way to + # silence the common case where the function type is the first + # template argument. False negative with less-than comparison is + # avoided because those operators are usually followed by a space. + # + # function // bracket + no space = false positive + # value < double(42) // bracket + space = true positive + matched_new_or_template = match.group(1) + + # Other things to ignore: + # - Function pointers + # - Casts to pointer types + # - Placement new + # - Alias declarations + matched_funcptr = match.group(3) + if (matched_new_or_template is None and + not (matched_funcptr and + (Match(r'\((?:[^() ]+::\s*\*\s*)?[^() ]+\)\s*\(', + matched_funcptr) or + matched_funcptr.startswith('(*)'))) and + not Match(r'\s*using\s+\S+\s*=\s*' + matched_type, line) and + not Search(r'new\(\S+\)\s*' + matched_type, line)): + error(filename, linenum, 'readability/casting', 4, + 'Using deprecated casting style. ' + 'Use static_cast<%s>(...) instead' % + matched_type) + + if not expecting_function: + CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], + 'static_cast', + r'\((int|float|double|bool|char|u?int(16|32|64))\)', error) + + # This doesn't catch all cases. Consider (const char * const)"hello". + # + # (char *) "foo" should always be a const_cast (reinterpret_cast won't + # compile). + if CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], + 'const_cast', r'\((char\s?\*+\s?)\)\s*"', error): + pass + else: + # Check pointer casts for other than string constants + CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], + 'reinterpret_cast', r'\((\w+\s?\*+\s?)\)', error) + + # In addition, we look for people taking the address of a cast. This + # is dangerous -- casts can assign to temporaries, so the pointer doesn't + # point where you think. + match = Search( + r'(?:&\(([^)]+)\)[\w(])|' + r'(?:&(static|dynamic|down|reinterpret)_cast\b)', line) + if match and match.group(1) != '*': + # Try a better error message when the & is bound to something + # dereferenced by the casted pointer, as opposed to the casted + # pointer itself. + parenthesis_error = False + match = Match(r'^(.*&(?:static|dynamic|down|reinterpret)_cast\b)<', line) + if match: + _, y1, x1 = CloseExpression(clean_lines, linenum, len(match.group(1))) + if x1 >= 0 and clean_lines.elided[y1][x1] == '(': + _, y2, x2 = CloseExpression(clean_lines, y1, x1) + if x2 >= 0: + extended_line = clean_lines.elided[y2][x2:] + if y2 < clean_lines.NumLines() - 1: + extended_line += clean_lines.elided[y2 + 1] + if Match(r'\s*(?:->|\[)', extended_line): + parenthesis_error = True + + if parenthesis_error: + error(filename, linenum, 'readability/casting', 4, + ('Are you taking an address of something dereferenced ' + 'from a cast? Wrapping the dereferenced expression in ' + 'parentheses will make the binding more obvious')) + else: + error(filename, linenum, 'runtime/casting', 4, + ('Are you taking an address of a cast? ' + 'This is dangerous: could be a temp var. ' + 'Take the address before doing the cast, rather than after')) def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern, @@ -4154,9 +4908,10 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern, if not match: return False - # Exclude lines with sizeof, since sizeof looks like a cast. - sizeof_match = Match(r'.*sizeof\s*$', line[0:match.start(1) - 1]) - if sizeof_match: + # Exclude lines with keywords that tend to look like casts, and also + # macros which are generally troublesome. + if Match(r'.*\b(?:sizeof|alignof|alignas|[A-Z_]+)\s*$', + line[0:match.start(1) - 1]): return False # operator++(int) and operator--(int) @@ -4188,7 +4943,8 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern, # ; # <(FunctionPointerTemplateArgument)(int)>; remainder = line[match.end(0):] - if Match(r'^\s*(?:;|const\b|throw\b|=|>|\{|\))', remainder): + 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. @@ -4226,6 +4982,28 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern, return True +def ExpectingFunctionArgs(clean_lines, linenum): + """Checks whether where function type arguments are expected. + + Args: + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + + Returns: + True if the line at 'linenum' is inside something that expects arguments + of function types. + """ + line = clean_lines.elided[linenum] + return (Match(r'^\s*MOCK_(CONST_)?METHOD\d+(_T)?\(', line) or + (linenum >= 2 and + (Match(r'^\s*MOCK_(?:CONST_)?METHOD\d+(?:_T)?\((?:\S+,)?\s*$', + clean_lines.elided[linenum - 1]) or + Match(r'^\s*MOCK_(?:CONST_)?METHOD\d+(?:_T)?\(\s*$', + clean_lines.elided[linenum - 2]) or + Search(r'\bstd::m?function\s*\<\s*$', + clean_lines.elided[linenum - 1])))) + + _HEADERS_CONTAINING_TEMPLATES = ( ('', ('deque',)), ('', ('unary_function', 'binary_function', @@ -4467,7 +5245,7 @@ _RE_PATTERN_EXPLICIT_MAKEPAIR = re.compile(r'\bmake_pair\s*<') def CheckMakePairUsesDeduction(filename, clean_lines, linenum, error): """Check that make_pair's template arguments are deduced. - G++ 4.6 in C++0x mode fails badly if make_pair's template arguments are + G++ 4.6 in C++11 mode fails badly if make_pair's template arguments are specified explicitly, and such use isn't intended in any case. Args: @@ -4483,11 +5261,35 @@ def CheckMakePairUsesDeduction(filename, clean_lines, linenum, error): 4, # 4 = high confidence 'For C++11-compatibility, omit template arguments from make_pair' ' 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 ProcessLine(filename, file_extension, clean_lines, line, - include_state, function_state, nesting_state, error, - extra_check_functions=[]): + include_state, function_state, nesting_state, error): """Processes a single line in the file. Args: @@ -4498,19 +5300,16 @@ def ProcessLine(filename, file_extension, clean_lines, line, line: Number of line being processed. include_state: An _IncludeState instance in which the headers are inserted. function_state: A _FunctionState instance which counts function lines, etc. - nesting_state: A _NestingState instance which maintains information about + nesting_state: A NestingState instance which maintains information about the current stack of nested blocks being parsed. error: A callable to which errors are reported, which takes 4 arguments: filename, line number, error level, and message - extra_check_functions: An array of additional check functions that will be - run on each source line. Each function takes 4 - arguments: filename, clean_lines, line, error + """ raw_lines = clean_lines.raw_lines ParseNolintSuppressions(filename, raw_lines[line], line, error) nesting_state.Update(filename, clean_lines, line, error) - if nesting_state.stack and nesting_state.stack[-1].inline_asm != _NO_ASM: - return + if nesting_state.InAsmBlock(): return CheckForFunctionLengths(filename, clean_lines, line, function_state, error) CheckForMultilineCommentsAndStrings(filename, clean_lines, line, error) CheckStyle(filename, clean_lines, line, file_extension, nesting_state, error) @@ -4523,11 +5322,59 @@ 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) - for check_fn in extra_check_functions: - check_fn(filename, clean_lines, line, error) + CheckDefaultLambdaCaptures(filename, clean_lines, line, error) -def ProcessFileData(filename, file_extension, lines, error, - extra_check_functions=[]): + +def FlagCxx11Features(filename, clean_lines, linenum, error): + """Flag those c++11 features that we only allow in certain places. + + 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] + + # Flag unapproved C++11 headers. + include = Match(r'\s*#\s*include\s+[<"]([^<"]+)[">]', line) + if include and include.group(1) in ('cfenv', + 'condition_variable', + 'fenv.h', + 'future', + 'mutex', + 'thread', + 'chrono', + 'ratio', + 'regex', + 'system_error', + ): + error(filename, linenum, 'build/c++11', 5, + ('<%s> is an unapproved C++11 header.') % include.group(1)) + + # The only place where we need to worry about C++11 keywords and library + # features in preprocessor directives is in macro definitions. + if Match(r'\s*#', line) and not Match(r'\s*#\s*define\b', line): return + + # These are classes and free functions. The classes are always + # mentioned as std::*, but we only catch the free functions if + # they're not found by ADL. They're alphabetical by header. + for top_name in ( + # type_traits + 'alignment_of', + 'aligned_union', + + # utility + 'forward', + ): + if Search(r'\bstd::%s\b' % top_name, line): + error(filename, linenum, 'build/c++11', 5, + ('std::%s is an unapproved C++11 class or function. Send c-style ' + 'an example of where it would make your code more readable, and ' + 'they may let you use it.') % top_name) + + +def ProcessFileData(filename, file_extension, lines, error): """Performs lint checks and reports any errors to the given error function. Args: @@ -4536,17 +5383,13 @@ def ProcessFileData(filename, file_extension, lines, error, 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. error: A callable to which errors are reported, which takes 4 arguments: - filename, line number, error level, and message - extra_check_functions: An array of additional check functions that will be - run on each source line. Each function takes 4 - arguments: filename, clean_lines, line, error """ lines = (['// marker so line numbers and indices both start at 1'] + lines + ['// marker so line numbers end in a known way']) include_state = _IncludeState() function_state = _FunctionState() - nesting_state = _NestingState() + nesting_state = NestingState() ResetNolintSuppressions() @@ -4559,8 +5402,8 @@ def ProcessFileData(filename, file_extension, lines, error, clean_lines = CleansedLines(lines) for line in xrange(clean_lines.NumLines()): ProcessLine(filename, file_extension, clean_lines, line, - include_state, function_state, nesting_state, error, - extra_check_functions) + include_state, function_state, nesting_state, error) + FlagCxx11Features(filename, clean_lines, line, error) nesting_state.CheckCompletedBlocks(filename, error) CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error) @@ -4571,7 +5414,8 @@ def ProcessFileData(filename, file_extension, lines, error, CheckForNewlineAtEOF(filename, lines, error) -def ProcessFile(filename, vlevel, extra_check_functions=[]): + +def ProcessFile(filename, vlevel): """Does google-lint on a single file. Args: @@ -4579,14 +5423,12 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): vlevel: The level of errors to report. Every error of confidence >= verbose_level will be reported. 0 is a good default. - - extra_check_functions: An array of additional check functions that will be - run on each source line. Each function takes 4 - arguments: filename, clean_lines, line, error """ _SetVerboseLevel(vlevel) + lf_lines = [] + crlf_lines = [] try: # Support the UNIX convention of using "-" for stdin. Note that # we are not opening the file with universal newline support @@ -4594,10 +5436,7 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): # contain trailing '\r' characters if we are reading a file that # has CRLF endings. # If after the split a trailing '\r' is present, it is removed - # below. If it is not expected to be present (i.e. os.linesep != - # '\r\n' as in Windows), a warning is issued below if this file - # is processed. - + # below. if filename == '-': lines = codecs.StreamReaderWriter(sys.stdin, codecs.getreader('utf8'), @@ -4606,12 +5445,14 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): else: lines = codecs.open(filename, 'r', 'utf8', 'replace').read().split('\n') - carriage_return_found = False # Remove trailing '\r'. - for linenum in range(len(lines)): + # The -1 accounts for the extra trailing blank line we get from split() + for linenum in range(len(lines) - 1): if lines[linenum].endswith('\r'): lines[linenum] = lines[linenum].rstrip('\r') - carriage_return_found = True + crlf_lines.append(linenum + 1) + else: + lf_lines.append(linenum + 1) except IOError: sys.stderr.write( @@ -4627,14 +5468,25 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): sys.stderr.write('Ignoring %s; not a valid file name ' '(%s)\n' % (filename, ', '.join(_valid_extensions))) else: - ProcessFileData(filename, file_extension, lines, Error, - extra_check_functions) - if carriage_return_found and os.linesep != '\r\n': - # Use 0 for linenum since outputting only one error for potentially - # several lines. - Error(filename, 0, 'whitespace/newline', 1, - 'One or more unexpected \\r (^M) found;' - 'better to use only a \\n') + ProcessFileData(filename, file_extension, lines, Error) + + # If end-of-line sequences are a mix of LF and CR-LF, issue + # warnings on the lines with CR. + # + # Don't issue any warnings if all lines are uniformly LF or CR-LF, + # since critique can handle these just fine, and the style guide + # doesn't dictate a particular end of line sequence. + # + # We can't depend on os.linesep to determine what the desired + # end-of-line sequence should be, since that will return the + # server-side end-of-line sequence. + if lf_lines and crlf_lines: + # Warn on every line with CR. An alternative approach might be to + # check whether the file is mostly CRLF or just LF, and warn on the + # minority, we bias toward LF here since most tools prefer LF. + for linenum in crlf_lines: + Error(filename, linenum, 'whitespace/newline', 1, + 'Unexpected \\r (^M) found; better to use only \\n') sys.stderr.write('Done processing %s\n' % filename) diff --git a/cpplint/cpplint_unittest.py b/cpplint/cpplint_unittest.py index 253bb80..8a66def 100755 --- a/cpplint/cpplint_unittest.py +++ b/cpplint/cpplint_unittest.py @@ -97,6 +97,7 @@ class ErrorCollector: # This class is a lame mock of codecs. We do not verify filename, mode, or # encoding, but for the current use case it is not needed. class MockIo: + def __init__(self, mock_file): self.mock_file = mock_file @@ -116,7 +117,7 @@ class CpplintTestBase(unittest.TestCase): clean_lines = cpplint.CleansedLines(lines) include_state = cpplint._IncludeState() function_state = cpplint._FunctionState() - nesting_state = cpplint._NestingState() + nesting_state = cpplint.NestingState() cpplint.ProcessLine('foo.cc', 'cc', clean_lines, 0, include_state, function_state, nesting_state, error_collector) @@ -132,7 +133,7 @@ class CpplintTestBase(unittest.TestCase): lines = code.split('\n') cpplint.RemoveMultiLineComments('foo.h', lines, error_collector) lines = cpplint.CleansedLines(lines) - nesting_state = cpplint._NestingState() + nesting_state = cpplint.NestingState() for i in xrange(lines.NumLines()): nesting_state.Update('foo.h', lines, i, error_collector) cpplint.CheckStyle('foo.h', lines, i, 'h', nesting_state, @@ -147,7 +148,7 @@ class CpplintTestBase(unittest.TestCase): def PerformLanguageRulesCheck(self, file_name, code): error_collector = ErrorCollector(self.assert_) include_state = cpplint._IncludeState() - nesting_state = cpplint._NestingState() + nesting_state = cpplint.NestingState() lines = code.split('\n') cpplint.RemoveMultiLineComments(file_name, lines, error_collector) lines = cpplint.CleansedLines(lines) @@ -187,7 +188,7 @@ class CpplintTestBase(unittest.TestCase): # First, build up the include state. error_collector = ErrorCollector(self.assert_) include_state = cpplint._IncludeState() - nesting_state = cpplint._NestingState() + nesting_state = cpplint.NestingState() lines = code.split('\n') cpplint.RemoveMultiLineComments(filename, lines, error_collector) lines = cpplint.CleansedLines(lines) @@ -262,7 +263,7 @@ class CpplintTest(CpplintTestBase): self.assertEquals('\n return loop(x);\n', cpplint._GetTextInside( 'int loop(int x) {\n return loop(x);\n}\n', r'\{')) - # '^' matches the beggining of each line. + # '^' matches the beginning of each line. self.assertEquals('x, y', cpplint._GetTextInside( '#include "inl.h" // skip #define\n' @@ -328,6 +329,22 @@ class CpplintTest(CpplintTestBase): '// $Id: g' + ('o' * 80) + 'gle.cc#1', 'Lines should be <= 80 characters long' ' [whitespace/line_length] [2]') + self.TestMultiLineLint( + 'static const char kCStr[] = "g' + ('o' * 50) + 'gle";\n', + 'Lines should be <= 80 characters long' + ' [whitespace/line_length] [2]') + self.TestMultiLineLint( + 'static const char kRawStr[] = R"(g' + ('o' * 50) + 'gle)";\n', + '') # no warning because raw string content is elided + self.TestMultiLineLint( + 'static const char kMultiLineRawStr[] = R"(\n' + 'g' + ('o' * 80) + 'gle\n' + ')";', + '') + self.TestMultiLineLint( + 'static const char kL' + ('o' * 50) + 'ngIdentifier[] = R"()";\n', + 'Lines should be <= 80 characters long' + ' [whitespace/line_length] [2]') # Test error suppression annotations. def testErrorSuppression(self): @@ -358,8 +375,7 @@ class CpplintTest(CpplintTestBase): self.TestLint( 'long a = 65; // NOLINT(readability/casting)', 'Use int16/int64/etc, rather than the C type long' - ' [runtime/int] [4]') - + ' [runtime/int] [4]') # Test Variable Declarations. def testVariableDeclarations(self): @@ -404,6 +420,11 @@ class CpplintTest(CpplintTestBase): self.TestLint('uint a = (uint)NULL;', '') self.TestLint('typedef MockCallback CallbackType;', '') self.TestLint('scoped_ptr< MockCallback > callback_value;', '') + self.TestLint('std::function', '') + self.TestLint('x = sizeof(int)', '') + self.TestLint('x = alignof(int)', '') + self.TestLint('alignas(int) char x[42]', '') + self.TestLint('alignas(alignof(x)) char y[42]', '') # Test taking address of casts (runtime/casting) def testRuntimeCasting(self): @@ -418,6 +439,18 @@ class CpplintTest(CpplintTestBase): 'instead [readability/casting] [4]', error_msg]) + # Alternative error message + alt_error_msg = ('Are you taking an address of something dereferenced ' + 'from a cast? Wrapping the dereferenced expression in ' + 'parentheses will make the binding more obvious' + ' [readability/casting] [4]') + self.TestLint('int* x = &down_cast(obj)->member_;', alt_error_msg) + self.TestLint('int* x = &down_cast(obj)[index];', alt_error_msg) + self.TestLint('int* x = &(down_cast(obj)->member_);', '') + self.TestLint('int* x = &(down_cast(obj)[index]);', '') + self.TestLint('int* x = &down_cast(obj)\n->member_;', alt_error_msg) + self.TestLint('int* x = &(down_cast(obj)\n->member_);', '') + # It's OK to cast an address. self.TestLint('int* x = reinterpret_cast(&foo);', '') @@ -441,13 +474,14 @@ class CpplintTest(CpplintTestBase): def testCheckForUnnamedParams(self): message = ('All parameters should be named in a function' ' [readability/function] [3]') - self.TestLint('virtual void A(int*) const;', message) - self.TestLint('virtual void B(int*);', message) + 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 D(int* p);', '') + self.TestLint('virtual void Func(int* p);', '') self.TestLint('void operator delete(void* x) throw();', '') self.TestLint('void Method(char* x) {', '') self.TestLint('void Method(char* /*x*/) {', '') @@ -460,10 +494,13 @@ class CpplintTest(CpplintTestBase): self.TestLint('X operator++(int) {', '') self.TestLint('X operator--(int);', '') self.TestLint('X operator--(int /*unused*/) {', '') + self.TestLint('MACRO(int);', '') self.TestLint('void (*func)(void*);', '') self.TestLint('void Func((*func)(void*)) {}', '') self.TestLint('template void func();', '') + self.TestLint('virtual void f(int /*unused*/) override {', '') + self.TestLint('virtual void f(int /*unused*/) final {', '') # Test deprecated casts such as int(d) def testDeprecatedCast(self): @@ -501,9 +538,34 @@ class CpplintTest(CpplintTestBase): self.TestLint( 'new int64(123); // "new" operator on basic type, weird spacing', '') + self.TestLint( + 'using a = bool(int arg); // C++11 alias-declaration', + '') self.TestLint( 'std::function // C++11 function wrapper', '') + self.TestLint( + 'std::function', + '') + self.TestLint( + 'std::function< int(bool) >', + '') + self.TestLint( + 'mfunction', + '') + self.TestLint( + 'new(field_ptr) int(field->default_value_enum()->number());', + '') + + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData( + 'test.cc', 'cc', + ['// Copyright 2008 Your Company. All Rights Reserved.', + 'typedef std::function<', + ' bool(int)> F;', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) # Return types for function pointers tend to look like casts. self.TestLint( @@ -533,6 +595,9 @@ class CpplintTest(CpplintTestBase): self.TestLint( 'bool TraverseNode(T *Node, bool(VisitorBase:: *traverse) (T *t)) {}', '') + self.TestLint( + 'x = bit_cast(y);', + '') # The second parameter to a gMock method definition is a function signature # that often looks like a bad cast but should not picked up by lint. @@ -546,6 +611,9 @@ class CpplintTest(CpplintTestBase): self.TestLint( 'MOCK_CONST_METHOD2_T(method, double(float, float));', '') + self.TestLint( + 'MOCK_CONST_METHOD1(method, SomeType(int));', + '') error_collector = ErrorCollector(self.assert_) cpplint.ProcessFileData('mock.cc', 'cc', @@ -573,7 +641,6 @@ class CpplintTest(CpplintTestBase): 'Use static_cast(...) instead ' '[readability/casting] [4]'))) - # Like gMock method definitions, MockCallback instantiations look very similar # to bad casts. def testMockCallback(self): @@ -835,6 +902,10 @@ class CpplintTest(CpplintTestBase): cpplint.CleanseComments('f(a /* name */, b);')) self.assertEqual('f(a, b);', cpplint.CleanseComments('f(a, /* name */b);')) + self.assertEqual('f(a, b, c);', + cpplint.CleanseComments('f(a, /**/b, /**/c);')) + self.assertEqual('f(a, b, c);', + cpplint.CleanseComments('f(a, /**/b/**/, c);')) def testRawStrings(self): self.TestMultiLineLint( @@ -880,6 +951,26 @@ class CpplintTest(CpplintTestBase): )";""", '') + self.TestMultiLineLint( + """ + void Func() { + string s = StrCat(R"TrueDelimiter( + )" + )FalseDelimiter" + )TrueDelimiter", R"TrueDelimiter2( + )" + )FalseDelimiter2" + )TrueDelimiter2"); + }""", + '') + self.TestMultiLineLint( + """ + static SomeStruct kData = { + {0, R"(line1 + line2 + )"} + };""", + '') def testMultiLineComments(self): # missing explicit is bad @@ -1075,6 +1166,13 @@ class CpplintTest(CpplintTestBase): Foo(const Foo&); };""", '') + # Special case for std::initializer_list + self.TestMultiLineLint( + """ + class Foo { + Foo(std::initializer_list &arg) {} + };""", + '') # Anything goes inside an assembly block error_collector = ErrorCollector(self.assert_) cpplint.ProcessFileData('foo.cc', 'cc', @@ -1179,18 +1277,20 @@ class CpplintTest(CpplintTestBase): self.TestLanguageRulesCheck('foo_unittest.cc', '#include ', '') def testCheckPosixThreading(self): - self.TestLint('sctime_r()', '') - self.TestLint('strtok_r()', '') - self.TestLint(' strtok_r(foo, ba, r)', '') - self.TestLint('brand()', '') + self.TestLint('var = sctime_r()', '') + self.TestLint('var = strtok_r()', '') + self.TestLint('var = strtok_r(foo, ba, r)', '') + self.TestLint('var = brand()', '') self.TestLint('_rand()', '') self.TestLint('.rand()', '') self.TestLint('->rand()', '') - self.TestLint('rand()', + self.TestLint('ACMRandom rand(seed)', '') + self.TestLint('ISAACRandom rand()', '') + self.TestLint('var = rand()', 'Consider using rand_r(...) instead of rand(...)' ' for improved thread safety.' ' [runtime/threadsafe_fn] [2]') - self.TestLint('strtok()', + self.TestLint('var = strtok(str, delim)', 'Consider using strtok_r(...) ' 'instead of strtok(...)' ' for improved thread safety.' @@ -1258,9 +1358,7 @@ class CpplintTest(CpplintTestBase): self.TestLint('void operator&(int a, int b)', '') # binary operator& ok self.TestLint('void operator&() { }', errmsg) self.TestLint('void operator & ( ) { }', - ['Extra space after ( [whitespace/parens] [2]', - errmsg - ]) + ['Extra space after ( [whitespace/parens] [2]', errmsg]) # const string reference members are dangerous.. def testConstStringReferenceMembers(self): @@ -1541,16 +1639,6 @@ class CpplintTest(CpplintTestBase): 'Consider using ASSERT_GE_M instead of ASSERT_FALSE_M(a < b)' ' [readability/check] [2]') - self.TestLint('CHECK(some_iterator == obj.end())', '') - self.TestLint('EXPECT_TRUE(some_iterator == obj.end())', '') - self.TestLint('EXPECT_FALSE(some_iterator == obj.end())', '') - self.TestLint('CHECK(some_pointer != NULL)', '') - self.TestLint('EXPECT_TRUE(some_pointer != NULL)', '') - self.TestLint('EXPECT_FALSE(some_pointer != NULL)', '') - - self.TestLint('CHECK(CreateTestFile(dir, (1 << 20)));', '') - self.TestLint('CHECK(CreateTestFile(dir, (1 >> 20)));', '') - self.TestLint('CHECK(x<42)', ['Missing spaces around <' ' [whitespace/operators] [3]', @@ -1564,11 +1652,6 @@ class CpplintTest(CpplintTestBase): self.TestLint('using some::namespace::operator<<;', '') self.TestLint('using some::namespace::operator>>;', '') - self.TestLint('CHECK(x ^ (y < 42))', '') - self.TestLint('CHECK((x > 42) ^ (x < 54))', '') - self.TestLint('CHECK(a && b < 42)', '') - self.TestLint('CHECK(42 < a && a < b)', '') - self.TestLint('SOFT_CHECK(x > 42)', '') self.TestLint('CHECK(x->y == 42)', 'Consider using CHECK_EQ instead of CHECK(a == b)' @@ -1584,12 +1667,41 @@ class CpplintTest(CpplintTestBase): ' [whitespace/parens] [4]', 'Consider using EXPECT_LT instead of EXPECT_TRUE(a < b)' ' [readability/check] [2]']) - self.TestLint( - 'CHECK("foo" == "foo")', - 'Consider using CHECK_EQ instead of CHECK(a == b)' - ' [readability/check] [2]') - self.TestLint('CHECK_EQ("foo", "foo")', '') + self.TestLint('CHECK(4\'2 == x)', + 'Consider using CHECK_EQ instead of CHECK(a == b)' + ' [readability/check] [2]') + + def testCheckCheckFalsePositives(self): + self.TestLint('CHECK(some_iterator == obj.end())', '') + self.TestLint('EXPECT_TRUE(some_iterator == obj.end())', '') + self.TestLint('EXPECT_FALSE(some_iterator == obj.end())', '') + self.TestLint('CHECK(some_pointer != NULL)', '') + self.TestLint('EXPECT_TRUE(some_pointer != NULL)', '') + self.TestLint('EXPECT_FALSE(some_pointer != NULL)', '') + + self.TestLint('CHECK(CreateTestFile(dir, (1 << 20)));', '') + self.TestLint('CHECK(CreateTestFile(dir, (1 >> 20)));', '') + + self.TestLint('CHECK(x ^ (y < 42))', '') + self.TestLint('CHECK((x > 42) ^ (x < 54))', '') + self.TestLint('CHECK(a && b < 42)', '') + self.TestLint('CHECK(42 < a && a < b)', '') + self.TestLint('SOFT_CHECK(x > 42)', '') + + self.TestMultiLineLint( + """_STLP_DEFINE_BINARY_OP_CHECK(==, _OP_EQUAL); + _STLP_DEFINE_BINARY_OP_CHECK(!=, _OP_NOT_EQUAL); + _STLP_DEFINE_BINARY_OP_CHECK(<, _OP_LESS_THAN); + _STLP_DEFINE_BINARY_OP_CHECK(<=, _OP_LESS_EQUAL); + _STLP_DEFINE_BINARY_OP_CHECK(>, _OP_GREATER_THAN); + _STLP_DEFINE_BINARY_OP_CHECK(>=, _OP_GREATER_EQUAL); + _STLP_DEFINE_BINARY_OP_CHECK(+, _OP_PLUS); + _STLP_DEFINE_BINARY_OP_CHECK(*, _OP_TIMES); + _STLP_DEFINE_BINARY_OP_CHECK(/, _OP_DIVIDE); + _STLP_DEFINE_BINARY_OP_CHECK(-, _OP_SUBTRACT); + _STLP_DEFINE_BINARY_OP_CHECK(%, _OP_MOD);""", + '') # Alternative token to punctuation operator replacements def testCheckAltTokens(self): @@ -1710,7 +1822,7 @@ class CpplintTest(CpplintTestBase): 'vector &nonconst_x) {', operand_error_message % 'vector &nonconst_x') - # Another potential false positive. This one needs full parser + # Other potential false positives. These need full parser # state to reproduce as opposed to just TestLint. error_collector = ErrorCollector(self.assert_) cpplint.ProcessFileData( @@ -1727,11 +1839,24 @@ class CpplintTest(CpplintTestBase): ' ostream& out', ' const dense_hash_set& seq) {', '}', + '#define UNSUPPORTED_MASK(_mask) \\', + ' if (flags & _mask) { \\', + ' LOG(FATAL) << "Unsupported flag: " << #_mask; \\', + ' }', + 'Constructor::Constructor()', + ' : initializer1_(a & b),', + ' initializer2_(a & b) {', + '}', + 'Constructor::Constructor()', + ' : initializer1_{a & b},', + ' initializer2_(a & b) {', + '}', ''], error_collector) self.assertEquals('', error_collector.Results()) # Multi-line references + error_collector = ErrorCollector(self.assert_) cpplint.ProcessFileData( 'foo.cc', 'cc', ['// Copyright 2008 Your Company. All Rights Reserved.', @@ -1756,6 +1881,22 @@ class CpplintTest(CpplintTestBase): operand_error_message % 'Outer::Inner& nonconst_z'], error_collector.Results()) + # A peculiar false positive due to bad template argument parsing + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData( + 'foo.cc', 'cc', + ['// Copyright 2008 Your Company. All Rights Reserved.', + 'inline RCULocked::ReadPtr::ReadPtr(const RCULocked* rcu) {', + ' DCHECK(!(data & kFlagMask)) << "Error";', + '}', + '', + 'RCULocked::WritePtr::WritePtr(RCULocked* rcu)', + ' : lock_(&rcu_->mutex_) {', + '}', + ''], + error_collector.Results()) + self.assertEquals('', error_collector.Results()) + def testBraceAtBeginOfLine(self): self.TestLint('{', '{ should almost always be at the end of the previous line' @@ -1847,6 +1988,9 @@ class CpplintTest(CpplintTestBase): self.TestLint('typedef foo (*foo)(foo)', '') self.TestLint('typedef foo (*foo12bar_)(foo)', '') self.TestLint('typedef foo (Foo::*bar)(foo)', '') + self.TestLint('using foo = type (Foo::*bar)(foo)', '') + self.TestLint('using foo = type (Foo::*bar)(', '') + self.TestLint('using foo = type (Foo::*)(', '') self.TestLint('foo (Foo::*bar)(', '') self.TestLint('foo (x::y::*z)(', '') self.TestLint('foo (Foo::bar)(', @@ -1900,6 +2044,40 @@ class CpplintTest(CpplintTestBase): self.TestLint('file_tocs_[i] = (FileToc) {a, b, c};', '') self.TestMultiLineLint('class X : public Y,\npublic Z {};', '') + def testLambda(self): + self.TestLint('auto x = []() {};', '') + self.TestLint('return []() {};', '') + self.TestMultiLineLint('auto x = []() {\n};\n', '') + self.TestLint('int operator[](int x) {};', + 'You don\'t need a ; after a } [readability/braces] [4]') + + self.TestMultiLineLint('auto x = [&a,\nb]() {};', '') + self.TestMultiLineLint('auto x = [&a,\nb]\n() {};', '') + self.TestMultiLineLint('auto x = [&a,\n' + ' b](\n' + ' int a,\n' + ' int b) {\n' + ' return a +\n' + ' b;\n' + '};\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);', '') + def testBraceInitializerList(self): self.TestLint('MyStruct p = {1, 2};', '') self.TestLint('MyStruct p{1, 2};', '') @@ -1919,6 +2097,8 @@ class CpplintTest(CpplintTestBase): self.TestLint('auto foo = std::unique_ptr{new Foo{}};', '') self.TestLint('static_assert(Max7String{}.IsValid(), "");', '') self.TestLint('map_of_pairs[{1, 2}] = 3;', '') + self.TestLint('ItemView{has_offer() ? new Offer{offer()} : nullptr', '') + self.TestLint('template {}> = 0>', '') self.TestMultiLineLint('std::unique_ptr foo{\n' ' new Foo{}\n' @@ -1943,13 +2123,15 @@ class CpplintTest(CpplintTestBase): self.TestLint('} else{', 'Missing space before {' ' [whitespace/braces] [5]') self.TestLint('} else {', '') - self.TestLint('} else if', '') + self.TestLint('} else if (foo) {', '') def testSpacingWithInitializerLists(self): self.TestLint('int v[1][3] = {{1, 2, 3}};', '') self.TestLint('int v[1][1] = {{0}};', '') def testSpacingForBinaryOps(self): + self.TestLint('if (foo||bar) {', 'Missing spaces around ||' + ' [whitespace/operators] [3]') self.TestLint('if (foo<=bar) {', 'Missing spaces around <=' ' [whitespace/operators] [3]') self.TestLint('if (foobar) {', 'Missing spaces around <' ' [whitespace/operators] [3]') self.TestLint('template', '') - self.TestLint('scoped_ptr>', '') + self.TestLint('typedef hash_map', '') self.TestLint('10<<20', '') self.TestLint('10<>* arg1);', '') self.TestLint('foo = new set>;', '') self.TestLint('reinterpret_cast>*>(a);', '') + self.TestLint('MACRO(<<)', '') + self.TestLint('MACRO(<<, arg)', '') + self.TestLint('MACRO(<<=)', '') + self.TestLint('MACRO(<<=, arg)', '') - # These don't warn because they have trailing commas. - self.TestLint('typedef hash_map::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 head in ['void Func', 'vector Func', 'vector\nFunc', + 'Constructor', 'Constructor::Constructor', + 'operator=', 'operator =', 'operator = ']: + for body in [' {}', ' {', ';']: + self.TestMultiLineLint(head + '(A&& b)' + body, rvalue_error) + self.TestMultiLineLint(head + '(A &&b)' + body, rvalue_error) + self.TestMultiLineLint(head + '(A&&... b)' + body, rvalue_error) + self.TestMultiLineLint(head + '(A&& c)' + body, rvalue_error) + self.TestMultiLineLint(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) + + # 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) + + # Verify that RValue reference warnings for a line range can be silenced + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('foo.cc', 'cc', + ['// Copyright 2008 Your Company.', + 'GOOGLE_ALLOW_RVALUE_REFERENCES_PUSH', + 'void F(A&& b);', + 'GOOGLE_ALLOW_RVALUE_REFERENCES_POP', + ''], + error_collector) + self.assertEquals(error_collector.Results(), '') def testSpacingBeforeLastSemicolon(self): self.TestLint('call_function() ;', @@ -2090,6 +2354,22 @@ class CpplintTest(CpplintTestBase): 'For a static/global string constant, use a C style ' 'string instead: "char Foo::bar[]".' ' [runtime/string] [4]') + self.TestLint('string* pointer', '') + self.TestLint('string *pointer', '') + self.TestLint('string* pointer = Func();', '') + self.TestLint('string *pointer = Func();', '') + self.TestLint('const string* pointer', '') + self.TestLint('const string *pointer', '') + self.TestLint('const string* pointer = Func();', '') + self.TestLint('const string *pointer = Func();', '') + self.TestLint('string const* pointer', '') + self.TestLint('string const *pointer', '') + self.TestLint('string const* pointer = Func();', '') + self.TestLint('string const *pointer = Func();', '') + self.TestLint('string* const pointer', '') + self.TestLint('string *const pointer', '') + self.TestLint('string* const pointer = Func();', '') + self.TestLint('string *const pointer = Func();', '') self.TestLint('string Foo::bar() {}', '') self.TestLint('string Foo::operator*() {}', '') # Rare case. @@ -2183,10 +2463,23 @@ class CpplintTest(CpplintTestBase): self.TestLint('int i = 0; // Having three spaces is OK.', '') self.TestLint('// Top level comment', '') self.TestLint(' // Line starts with two spaces.', '') - self.TestLint('foo();\n' - '{ // A scope is opening.', '') - self.TestLint(' foo();\n' - ' { // An indented scope is opening.', '') + self.TestMultiLineLint('void foo() {\n' + ' { // A scope is opening.\n' + ' int a;', '') + self.TestMultiLineLint('void foo() {\n' + ' { // A scope is opening.\n' + '#define A a', + 'At least two spaces is best between code and ' + 'comments [whitespace/comments] [2]') + self.TestMultiLineLint(' foo();\n' + ' { // An indented scope is opening.\n' + ' int a;', '') + self.TestMultiLineLint('vector my_elements = {// first\n' + ' 1,', '') + self.TestMultiLineLint('vector my_elements = {// my_elements is ..\n' + ' 1,', + 'At least two spaces is best between code and ' + 'comments [whitespace/comments] [2]') self.TestLint('if (foo) { // not a pure scope; comment is too close!', 'At least two spaces is best between code and comments' ' [whitespace/comments] [2]') @@ -2209,14 +2502,13 @@ class CpplintTest(CpplintTestBase): self.TestLint('///', '') # Empty Doxygen comment self.TestLint('////x', 'Should have a space between // and comment' ' [whitespace/comments] [4]') + self.TestLint('//}', '') + self.TestLint('//}x', 'Should have a space between // and comment' + ' [whitespace/comments] [4]') self.TestLint('//! + static void Func() { + }""", + '') + + def testConditionals(self): + self.TestMultiLineLint( + """ + if (foo) + goto fail; + goto fail;""", + 'If/else bodies with multiple statements require braces' + ' [readability/braces] [4]') + self.TestMultiLineLint( + """ + if (foo) + goto fail; goto fail;""", + 'If/else bodies with multiple statements require braces' + ' [readability/braces] [4]') + self.TestMultiLineLint( + """ + if (foo) + foo; + else + goto fail; + goto fail;""", + 'If/else bodies with multiple statements require braces' + ' [readability/braces] [4]') + self.TestMultiLineLint( + """ + if (foo) goto fail; + goto fail;""", + 'If/else bodies with multiple statements require braces' + ' [readability/braces] [4]') + self.TestMultiLineLint( + """ + if (foo) + if (bar) + baz; + else + qux;""", + 'Else clause should be indented at the same level as if. Ambiguous' + ' nested if/else chains require braces. [readability/braces] [4]') + self.TestMultiLineLint( + """ + if (foo) + if (bar) + baz; + else + qux;""", + 'Else clause should be indented at the same level as if. Ambiguous' + ' nested if/else chains require braces. [readability/braces] [4]') + self.TestMultiLineLint( + """ + if (foo) { + bar; + baz; + } else + qux;""", + 'If an else has a brace on one side, it should have it on both' + ' [readability/braces] [5]') + self.TestMultiLineLint( + """ + if (foo) + bar; + else { + baz; + }""", + 'If an else has a brace on one side, it should have it on both' + ' [readability/braces] [5]') + self.TestMultiLineLint( + """ + if (foo) + bar; + else if (baz) { + qux; + }""", + 'If an else has a brace on one side, it should have it on both' + ' [readability/braces] [5]') + self.TestMultiLineLint( + """ + if (foo) { + bar; + } else if (baz) + qux;""", + 'If an else has a brace on one side, it should have it on both' + ' [readability/braces] [5]') + self.TestMultiLineLint( + """ + if (foo) + goto fail; + bar;""", + '') + self.TestMultiLineLint( + """ + if (foo + && bar) { + baz; + qux; + }""", + '') + self.TestMultiLineLint( + """ + if (foo) + goto + fail;""", + '') + self.TestMultiLineLint( + """ + if (foo) + bar; + else + baz; + qux;""", + '') + self.TestMultiLineLint( + """ + for (;;) { + if (foo) + bar; + else + baz; + }""", + '') + self.TestMultiLineLint( + """ + if (foo) + bar; + else if (baz) + baz;""", + '') + self.TestMultiLineLint( + """ + if (foo) + bar; + else + baz;""", + '') + self.TestMultiLineLint( + """ + if (foo) { + bar; + } else { + baz; + }""", + '') + self.TestMultiLineLint( + """ + if (foo) { + bar; + } else if (baz) { + qux; + }""", + '') + # Note: this is an error for a different reason, but should not trigger the + # single-line if error. + self.TestMultiLineLint( + """ + if (foo) + { + bar; + baz; + }""", + '{ should almost always be at the end of the previous line' + ' [whitespace/braces] [4]') + self.TestMultiLineLint( + """ + if (foo) { \\ + bar; \\ + baz; \\ + }""", + '') + self.TestMultiLineLint( + """ + void foo() { if (bar) baz; }""", + '') + self.TestMultiLineLint( + """ + #if foo + bar; + #else + baz; + qux; + #endif""", + '') def testTab(self): self.TestLint('\tint a;', @@ -3038,29 +3603,30 @@ class CpplintTest(CpplintTestBase): ' [build/include] [4]') def testBuildPrintfFormat(self): - self.TestLint( - r'printf("\%%d", value);', - '%, [, (, and { are undefined character escapes. Unescape them.' - ' [build/printf_format] [3]') + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData( + 'foo.cc', 'cc', + [r'printf("\%%d", value);', + r'snprintf(buffer, sizeof(buffer), "\[%d", value);', + r'fprintf(file, "\(%d", value);', + r'vsnprintf(buffer, sizeof(buffer), "\\\{%d", ap);'], + error_collector) + self.assertEquals( + 4, + error_collector.Results().count( + '%, [, (, and { are undefined character escapes. Unescape them.' + ' [build/printf_format] [3]')) - self.TestLint( - r'snprintf(buffer, sizeof(buffer), "\[%d", value);', - '%, [, (, and { are undefined character escapes. Unescape them.' - ' [build/printf_format] [3]') - - self.TestLint( - r'fprintf(file, "\(%d", value);', - '%, [, (, and { are undefined character escapes. Unescape them.' - ' [build/printf_format] [3]') - - self.TestLint( - r'vsnprintf(buffer, sizeof(buffer), "\\\{%d", ap);', - '%, [, (, and { are undefined character escapes. Unescape them.' - ' [build/printf_format] [3]') - - # Don't warn if double-slash precedes the symbol - self.TestLint(r'printf("\\%%%d", value);', - '') + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData( + 'foo.cc', 'cc', + ['// Copyright 2008 Your Company.', + r'printf("\\%%%d", value);', + r'printf(R"(\[)");', + r'printf(R"(\[%s)", R"(\])");', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) def testRuntimePrintfFormat(self): self.TestLint( @@ -3211,6 +3777,65 @@ class CpplintTest(CpplintTestBase): self.TestLint('snprintf(fisk, 1, format)', 'If you can, use sizeof(fisk) instead of 1 as the 2nd arg ' 'to snprintf. [runtime/printf] [3]') +class Cxx11Test(CpplintTestBase): + + def Helper(self, package, extension, lines, count): + filename = package + '/foo.' + extension + lines = lines[:] + + # Header files need to have an ifdef guard wrapped around their code. + if extension == 'h': + guard = filename.upper().replace('/', '_').replace('.', '_') + '_' + lines.insert(0, '#ifndef ' + guard) + lines.insert(1, '#define ' + guard) + lines.append('#endif // ' + guard) + + # All files need a final blank line. + lines.append('') + + # Process the file and check resulting error count. + collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData(filename, extension, lines, collector) + error_list = collector.ResultList() + self.assertEquals(count, len(error_list), error_list) + + def TestCxx11Feature(self, code, expected_error): + lines = code.split('\n') + collector = ErrorCollector(self.assert_) + cpplint.RemoveMultiLineComments('foo.h', lines, collector) + clean_lines = cpplint.CleansedLines(lines) + cpplint.FlagCxx11Features('foo.cc', clean_lines, 0, collector) + self.assertEquals(expected_error, collector.Results()) + + def testBlockedHeaders(self): + self.TestCxx11Feature('#include ', + ' is an unapproved C++11 header.' + ' [build/c++11] [5]') + + def testBlockedClasses(self): + self.TestCxx11Feature('std::alignment_of', + 'std::alignment_of is an unapproved ' + 'C++11 class or function. Send c-style an example ' + 'of where it would make your code more readable, ' + 'and they may let you use it.' + ' [build/c++11] [5]') + self.TestCxx11Feature('std::alignment_offer', '') + self.TestCxx11Feature('mystd::alignment_of', '') + self.TestCxx11Feature('std::binomial_distribution', '') + + def testBlockedFunctions(self): + self.TestCxx11Feature('std::alignment_of', + 'std::alignment_of is an unapproved ' + 'C++11 class or function. Send c-style an example ' + 'of where it would make your code more readable, ' + 'and they may let you use it.' + ' [build/c++11] [5]') + # Missed because of the lack of "std::". Compiles because ADL + # looks in the namespace of my_shared_ptr, which (presumably) is + # std::. But there will be a lint error somewhere in this file + # since my_shared_ptr had to be defined. + self.TestCxx11Feature('static_pointer_cast(my_shared_ptr)', '') + self.TestCxx11Feature('std::declval()', '') def testExplicitMakePair(self): self.TestLint('make_pair', '') @@ -3228,6 +3853,7 @@ class CpplintTest(CpplintTestBase): self.TestLint('my_make_pair', '') class CleansedLinesTest(unittest.TestCase): + def testInit(self): lines = ['Line 1', 'Line 2', @@ -3278,7 +3904,22 @@ class CleansedLinesTest(unittest.TestCase): self.assertEquals('', collapse('\\012')) # '\012' (char) self.assertEquals('', collapse('\\xfF0')) # '\xfF0' (char) self.assertEquals('', collapse('\\n')) # '\n' (char) - self.assertEquals('\#', collapse('\\#')) # '\#' (bad) + self.assertEquals(r'\#', collapse('\\#')) # '\#' (bad) + + self.assertEquals('"" + ""', collapse('"\'" + "\'"')) + self.assertEquals("'', ''", collapse("'\"', '\"'")) + self.assertEquals('""[0b10]', collapse('"a\'b"[0b1\'0]')) + + self.assertEquals('42', collapse("4'2")) + self.assertEquals('0b0101', collapse("0b0'1'0'1")) + self.assertEquals('1048576', collapse("1'048'576")) + self.assertEquals('0X100000', collapse("0X10'0000")) + self.assertEquals('0004000000', collapse("0'004'000'000")) + self.assertEquals('1.602176565e-19', collapse("1.602'176'565e-19")) + self.assertEquals('\'\' + 0xffff', collapse("'i' + 0xf'f'f'f")) + self.assertEquals('sizeof\'\' == 1', collapse("sizeof'x' == 1")) + self.assertEquals('0x.03p100', collapse('0x.0\'3p1\'0\'0')) + self.assertEquals('123.45', collapse('1\'23.4\'5')) self.assertEquals('StringReplace(body, "", "");', collapse('StringReplace(body, "\\\\", "\\\\\\\\");')) @@ -3287,6 +3928,7 @@ class CleansedLinesTest(unittest.TestCase): class OrderOfIncludesTest(CpplintTestBase): + def setUp(self): self.include_state = cpplint._IncludeState() # Cheat os.path.abspath called in FileInfo class. @@ -3408,6 +4050,7 @@ class OrderOfIncludesTest(CpplintTestBase): '"foo/foo-inl.h"', '', '', + '', '"bar/bar-inl.h"', '"bar/bar.h"']), '') @@ -3513,6 +4156,7 @@ class OrderOfIncludesTest(CpplintTestBase): class CheckForFunctionLengthsTest(CpplintTestBase): + def setUp(self): # Reducing these thresholds for the tests speeds up tests significantly. self.old_normal_trigger = cpplint._FunctionState._NORMAL_TRIGGER @@ -3822,9 +4466,107 @@ class CheckForFunctionLengthsTest(CpplintTestBase): 'Lint failed to find start of function body.' ' [readability/fn_size] [5]') -class NestnigStateTest(unittest.TestCase): + +def TrimExtraIndent(text_block): + """Trim a uniform amount of whitespace off of each line in a string. + + Compute the minimum indent on all non blank lines and trim that from each, so + that the block of text has no extra indentation. + + Args: + text_block: a multiline string + + Returns: + text_block with the common whitespace indent of each line removed. + """ + + def CountLeadingWhitespace(s): + count = 0 + for c in s: + if not c.isspace(): + break + count += 1 + return count + # find the minimum indent (except for blank lines) + min_indent = min([CountLeadingWhitespace(line) + for line in text_block.split('\n') if line]) + return '\n'.join([line[min_indent:] for line in text_block.split('\n')]) + + +class CloseExpressionTest(unittest.TestCase): + def setUp(self): - self.nesting_state = cpplint._NestingState() + self.lines = cpplint.CleansedLines( + # 1 2 3 4 5 + # 0123456789012345678901234567890123456789012345678901234567890 + ['// Line 0', + 'inline RCULocked::ReadPtr::ReadPtr(const RCULocked* rcu) {', + ' DCHECK(!(data & kFlagMask)) << "Error";', + '}', + '// Line 4', + 'RCULocked::WritePtr::WritePtr(RCULocked* rcu)', + ' : lock_(&rcu_->mutex_) {', + '}', + '// Line 8', + 'template ', + 'typename std::enable_if<', + ' std::is_array::value && (std::extent::value > 0)>::type', + 'MakeUnique(A&&... a) = delete;', + '// Line 13', + 'auto x = []() {};', + '// Line 15', + 'template ', + 'friend bool operator==(const reffed_ptr& a,', + ' const reffed_ptr& b) {', + ' return a.get() == b.get();', + '}', + '// Line 21']) + + def testCloseExpression(self): + # List of positions to test: + # (start line, start position, end line, end position + 1) + positions = [(1, 16, 1, 19), + (1, 37, 1, 59), + (1, 60, 3, 1), + (2, 8, 2, 29), + (2, 30, 22, -1), # Left shift operator + (9, 9, 9, 36), + (10, 23, 11, 59), + (11, 54, 22, -1), # Greater than operator + (14, 9, 14, 11), + (14, 11, 14, 13), + (14, 14, 14, 16), + (17, 22, 18, 46), + (18, 47, 20, 1)] + for p in positions: + (_, line, column) = cpplint.CloseExpression(self.lines, p[0], p[1]) + self.assertEquals((p[2], p[3]), (line, column)) + + def testReverseCloseExpression(self): + # List of positions to test: + # (end line, end position, start line, start position) + positions = [(1, 18, 1, 16), + (1, 58, 1, 37), + (2, 27, 2, 10), + (2, 28, 2, 8), + (6, 18, 0, -1), # -> operator + (9, 35, 9, 9), + (11, 54, 0, -1), # Greater than operator + (11, 57, 11, 31), + (14, 10, 14, 9), + (14, 12, 14, 11), + (14, 15, 14, 14), + (18, 45, 17, 22), + (20, 0, 18, 47)] + for p in positions: + (_, line, column) = cpplint.ReverseCloseExpression(self.lines, p[0], p[1]) + self.assertEquals((p[2], p[3]), (line, column)) + + +class NestingStateTest(unittest.TestCase): + + def setUp(self): + self.nesting_state = cpplint.NestingState() self.error_collector = ErrorCollector(self.assert_) def UpdateWithLines(self, lines): @@ -4035,6 +4777,23 @@ class NestnigStateTest(unittest.TestCase): self.assertTrue(isinstance(self.nesting_state.stack[0], cpplint._ClassInfo)) self.assertEquals(self.nesting_state.stack[0].name, 'D') + self.UpdateWithLines(['{', '};']) + self.assertEquals(len(self.nesting_state.stack), 0) + + self.UpdateWithLines(['template ', + 'static void Func() {']) + self.assertEquals(len(self.nesting_state.stack), 1) + self.assertFalse(isinstance(self.nesting_state.stack[0], + cpplint._ClassInfo)) + self.UpdateWithLines(['}', + 'template class K {']) + self.assertEquals(len(self.nesting_state.stack), 1) + self.assertTrue(isinstance(self.nesting_state.stack[0], cpplint._ClassInfo)) + self.assertEquals(self.nesting_state.stack[0].name, 'K') + def testTemplateInnerClass(self): self.UpdateWithLines(['class A {', ' public:'])