diff options
author | Elliott Hughes <enh@google.com> | 2012-06-21 10:41:20 -0700 |
---|---|---|
committer | Elliott Hughes <enh@google.com> | 2012-06-21 10:41:57 -0700 |
commit | db385708bfcfbd64bdc9cb46fdc0e66344363848 (patch) | |
tree | 3e2172c857a6bae61bba54587e8fcdc5bbd65b6d | |
parent | 5937e047bd15b3ce48c95e0457e501ab6feb7234 (diff) | |
download | android_art-db385708bfcfbd64bdc9cb46fdc0e66344363848.tar.gz android_art-db385708bfcfbd64bdc9cb46fdc0e66344363848.tar.bz2 android_art-db385708bfcfbd64bdc9cb46fdc0e66344363848.zip |
Update cpplint.py.
From https://code.google.com/p/google-styleguide/source/browse/trunk#trunk%2Fcpplint .
Change-Id: Ie9c0efe36c9af5c1fead502ae673ca6693f3cf7b
-rwxr-xr-x | tools/cpplint.py | 411 |
1 files changed, 323 insertions, 88 deletions
diff --git a/tools/cpplint.py b/tools/cpplint.py index 73a4644aa1..526b9556dc 100755 --- a/tools/cpplint.py +++ b/tools/cpplint.py @@ -150,6 +150,7 @@ _ERROR_CATEGORIES = [ 'build/class', 'build/deprecated', 'build/endif_comment', + 'build/explicit_make_pair', 'build/forward_decl', 'build/header_guard', 'build/include', @@ -210,11 +211,11 @@ _ERROR_CATEGORIES = [ # 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. -_DEFAULT_FILTERS = [ '-build/include_alpha' ] +_DEFAULT_FILTERS = ['-build/include_alpha'] # We used to check for high-bit characters, but after much discussion we # decided those were OK, as long as they were in UTF-8 and didn't represent -# hard-coded international strings, which belong in a seperate i18n file. +# hard-coded international strings, which belong in a separate i18n file. # Headers that we consider STL headers. _STL_HEADERS = frozenset([ @@ -235,11 +236,11 @@ _CPP_HEADERS = frozenset([ 'cstdio', 'cstdlib', 'cstring', 'ctime', 'cwchar', 'cwctype', 'defalloc.h', 'deque.h', 'editbuf.h', 'exception', 'fstream', 'fstream.h', 'hashtable.h', 'heap.h', 'indstream.h', 'iomanip', - 'iomanip.h', 'ios', 'iosfwd', 'iostream', 'iostream.h', 'istream.h', - 'iterator.h', 'limits', 'map.h', 'multimap.h', 'multiset.h', - 'numeric', 'ostream.h', 'parsestream.h', 'pfstream.h', 'PlotFile.h', - 'procbuf.h', 'pthread_alloc.h', 'rope', 'rope.h', 'ropeimpl.h', - 'SFile.h', 'slist', 'slist.h', 'stack.h', 'stdexcept', + 'iomanip.h', 'ios', 'iosfwd', 'iostream', 'iostream.h', 'istream', + 'istream.h', 'iterator.h', 'limits', 'map.h', 'multimap.h', 'multiset.h', + 'numeric', 'ostream', 'ostream.h', 'parsestream.h', 'pfstream.h', + 'PlotFile.h', 'procbuf.h', 'pthread_alloc.h', 'rope', 'rope.h', + 'ropeimpl.h', 'SFile.h', 'slist', 'slist.h', 'stack.h', 'stdexcept', 'stdiostream.h', 'streambuf.h', 'stream.h', 'strfile.h', 'string', 'strstream', 'strstream.h', 'tempbuf.h', 'tree.h', 'typeinfo', 'valarray', ]) @@ -310,9 +311,9 @@ def ParseNolintSuppressions(filename, raw_line, linenum, error): error: function, an error handler. """ # FIXME(adonovan): "NOLINT(" is misparsed as NOLINT(*). - m = _RE_SUPPRESSION.search(raw_line) - if m: - category = m.group(1) + matched = _RE_SUPPRESSION.search(raw_line) + if matched: + category = matched.group(1) if category in (None, '(*)'): # => "suppress all" _error_suppressions.setdefault(None, set()).add(linenum) else: @@ -322,7 +323,7 @@ def ParseNolintSuppressions(filename, raw_line, linenum, error): _error_suppressions.setdefault(category, set()).add(linenum) else: error(filename, linenum, 'readability/nolint', 5, - 'Unknown NOLINT error category: %s' % category) + 'Unknown NOLINT error category: %s' % category) def ResetNolintSuppressions(): @@ -404,7 +405,7 @@ class _IncludeState(dict): self._last_header = '' def CanonicalizeAlphabeticalOrder(self, header_path): - """Returns a path canonicalized for alphabetical comparisson. + """Returns a path canonicalized for alphabetical comparison. - replaces "-" with "_" so they both cmp the same. - removes '-inl' since we don't require them to be after the main header. @@ -662,7 +663,7 @@ class _FunctionState(object): self.current_function, self.lines_in_function, trigger)) def End(self): - """Stop analizing function body.""" + """Stop analyzing function body.""" self.in_a_function = False @@ -712,16 +713,18 @@ class FileInfo: prefix = os.path.commonprefix([root_dir, project_dir]) return fullname[len(prefix) + 1:] - # Not SVN? Try to find a git or hg top level directory by searching up - # from the current path. + # Not SVN <= 1.6? Try to find a git, hg, or svn top level directory by + # searching up from the current path. root_dir = os.path.dirname(fullname) while (root_dir != os.path.dirname(root_dir) and not os.path.exists(os.path.join(root_dir, ".git")) and - not os.path.exists(os.path.join(root_dir, ".hg"))): + not os.path.exists(os.path.join(root_dir, ".hg")) and + not os.path.exists(os.path.join(root_dir, ".svn"))): root_dir = os.path.dirname(root_dir) if (os.path.exists(os.path.join(root_dir, ".git")) or - os.path.exists(os.path.join(root_dir, ".hg"))): + os.path.exists(os.path.join(root_dir, ".hg")) or + os.path.exists(os.path.join(root_dir, ".svn"))): prefix = os.path.commonprefix([root_dir, project_dir]) return fullname[len(prefix) + 1:] @@ -760,8 +763,7 @@ class FileInfo: def _ShouldPrintError(category, confidence, linenum): - """Returns true iff confidence >= verbose, category passes - filter and is not NOLINT-suppressed.""" + """If confidence >= verbose, category passes filter and is not suppressed.""" # There are three ways we might decide not to print an error message: # a "NOLINT(category)" comment appears in the source, @@ -913,7 +915,7 @@ def CleanseComments(line): """ commentpos = line.find('//') if commentpos != -1 and not IsCppString(line[:commentpos]): - line = line[:commentpos] + line = line[:commentpos].rstrip() # get rid of /* ... */ return _RE_PATTERN_CLEANSE_LINE_C_COMMENTS.sub('', line) @@ -966,7 +968,7 @@ class CleansedLines(object): def CloseExpression(clean_lines, linenum, pos): """If input points to ( or { or [, finds the position that closes it. - If lines[linenum][pos] points to a '(' or '{' or '[', finds the the + If lines[linenum][pos] points to a '(' or '{' or '[', finds the linenum/pos that correspond to the closing of the expression. Args: @@ -1069,12 +1071,18 @@ def CheckForHeaderGuard(filename, lines, error): endif = line endif_linenum = linenum - if not ifndef or not define or ifndef != define: + if not ifndef: error(filename, 0, 'build/header_guard', 5, 'No #ifndef header guard found, suggested CPP variable is: %s' % cppvar) return + if not define: + error(filename, 0, 'build/header_guard', 5, + 'No #define header guard found, suggested CPP variable is: %s' % + cppvar) + return + # The guard should be PATH_FILE_H_, but we also allow PATH_FILE_H__ # for backward compatibility. if ifndef != cppvar: @@ -1087,6 +1095,12 @@ def CheckForHeaderGuard(filename, lines, error): error(filename, ifndef_linenum, 'build/header_guard', error_level, '#ifndef header guard has wrong style, please use: %s' % cppvar) + if define != ifndef: + error(filename, 0, 'build/header_guard', 5, + '#ifndef and #define don\'t match, suggested CPP variable is: %s' % + cppvar) + return + if endif != ('#endif // %s' % cppvar): error_level = 0 if endif != ('#endif // %s' % (cppvar + '_')): @@ -1248,7 +1262,7 @@ def CheckInvalidIncrement(filename, clean_lines, linenum, error): class _ClassInfo(object): """Stores information about a class.""" - def __init__(self, name, linenum): + def __init__(self, name, clean_lines, linenum): self.name = name self.linenum = linenum self.seen_open_brace = False @@ -1257,6 +1271,20 @@ class _ClassInfo(object): self.has_virtual_destructor = False self.brace_depth = 0 + # Try to find the end of the class. This will be confused by things like: + # class A { + # } *x = { ... + # + # But it's still good enough for CheckSectionSpacing. + self.last_line = 0 + depth = 0 + for i in range(linenum, clean_lines.NumLines()): + line = clean_lines.lines[i] + depth += line.count('{') - line.count('}') + if not depth: + self.last_line = i + break + class _ClassState(object): """Holds the current state of the parse relating to class declarations. @@ -1378,11 +1406,16 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, # style guidelines, but it seems to perform well enough in testing # to be a worthwhile addition to the checks. classinfo_stack = class_state.classinfo_stack - # Look for a class declaration + # Look for a class declaration. The regexp accounts for decorated classes + # such as in: + # class LOCKABLE API Object { + # }; class_decl_match = Match( - r'\s*(template\s*<[\w\s<>,:]*>\s*)?(class|struct)\s+(\w+(::\w+)*)', line) + r'\s*(template\s*<[\w\s<>,:]*>\s*)?' + '(class|struct)\s+([A-Z_]+\s+)*(\w+(::\w+)*)', line) if class_decl_match: - classinfo_stack.append(_ClassInfo(class_decl_match.group(3), linenum)) + classinfo_stack.append(_ClassInfo( + class_decl_match.group(4), clean_lines, linenum)) # Everything else in this function uses the top of the stack if it's # not empty. @@ -1412,12 +1445,12 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, # Look for single-argument constructors that aren't marked explicit. # Technically a valid construct, but against style. - args = Match(r'(?<!explicit)\s+%s\s*\(([^,()]+)\)' + args = Match(r'\s+(?:inline\s+)?%s\s*\(([^,()]+)\)' % re.escape(base_classname), line) if (args and args.group(1) != 'void' and - not Match(r'(const\s+)?%s\s*&' % re.escape(base_classname), + not Match(r'(const\s+)?%s\s*(?:<\w+>\s*)?&' % re.escape(base_classname), args.group(1).strip())): error(filename, linenum, 'runtime/explicit', 5, 'Single-argument constructors should be marked explicit.') @@ -1508,8 +1541,14 @@ def CheckSpacingForFunctionCall(filename, line, linenum, error): # 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): - error(filename, linenum, 'whitespace/parens', 2, - 'Extra space before )') + # If the closing parenthesis is preceded by only whitespaces, + # try to give a more descriptive error message. + if Search(r'^\s+\)', fncall): + error(filename, linenum, 'whitespace/parens', 2, + 'Closing ) should be moved to the previous line') + else: + error(filename, linenum, 'whitespace/parens', 2, + 'Extra space before )') def IsBlankLine(line): @@ -1540,7 +1579,7 @@ def CheckForFunctionLengths(filename, clean_lines, linenum, Trivial bodies are unchecked, so constructors with huge initializer lists may be missed. Blank/comment lines are not counted so as to avoid encouraging the removal - of vertical space and commments just to get through a lint check. + of vertical space and comments just to get through a lint check. NOLINT *on the last line of a function* disables this check. Args: @@ -1636,8 +1675,8 @@ def CheckSpacing(filename, clean_lines, linenum, error): Things we check for: spaces around operators, spaces after if/for/while/switch, no spaces around parens in function calls, two spaces between code and comment, don't start a block with a blank - line, don't end a function with a blank line, don't have too many - blank lines in a row. + line, don't end a function with a blank line, don't add a blank line + after public/protected/private, don't have too many blank lines in a row. Args: filename: The name of the current file. @@ -1664,7 +1703,7 @@ def CheckSpacing(filename, clean_lines, linenum, error): and prev_line[:prevbrace].find('namespace') == -1): # OK, we have a blank line at the start of a code block. Before we # complain, we check if it is an exception to the rule: The previous - # non-empty line has the paramters of a function header that are indented + # non-empty line has the parameters of a function header that are indented # 4 spaces (because they did not fit in a 80 column line when placed on # the same line as the function name). We also check for the case where # the previous line is indented 6 spaces, which may happen when the @@ -1715,6 +1754,11 @@ def CheckSpacing(filename, clean_lines, linenum, error): error(filename, linenum, 'whitespace/blank_line', 3, 'Blank line at the end of a code block. Is this needed?') + matched = Match(r'\s*(public|protected|private):', prev_line) + if matched: + 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: @@ -1824,13 +1868,22 @@ def CheckSpacing(filename, clean_lines, linenum, error): error(filename, linenum, 'whitespace/comma', 3, 'Missing space after ,') + # You should always have a space after a semicolon + # except for few corner cases + # TODO(unknown): clarify if 'if (1) { return 1;}' is requires one more + # space after ; + if Search(r';[^\s};\\)/]', line): + error(filename, linenum, 'whitespace/semicolon', 3, + 'Missing space after ;') + # Next we will look for issues with function calls. CheckSpacingForFunctionCall(filename, line, linenum, error) - # Except after an opening paren, you should have spaces before your braces. - # And since you should never have braces at the beginning of a line, this is - # an easy test. - if Search(r'[^ (]{', line): + # Except after an opening paren, or after another opening brace (in case of + # an initializer list, for instance), you should have spaces before your + # braces. And since you should never have braces at the beginning of a line, + # this is an easy test. + if Search(r'[^ ({]{', line): error(filename, linenum, 'whitespace/braces', 5, 'Missing space before {') @@ -1862,6 +1915,58 @@ def CheckSpacing(filename, clean_lines, linenum, error): 'statement, use { } instead.') +def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error): + """Checks for additional blank line issues related to sections. + + Currently the only thing checked here is blank line before protected/private. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + class_info: A _ClassInfo objects. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + # Skip checks if the class is small, where small means 25 lines or less. + # 25 lines seems like a good cutoff since that's the usual height of + # terminals, and any class that can't fit in one screen can't really + # be considered "small". + # + # Also skip checks if we are on the first line. This accounts for + # classes that look like + # class Foo { public: ... }; + # + # If we didn't find the end of the class, last_line would be zero, + # and the check will be skipped by the first condition. + if (class_info.last_line - class_info.linenum <= 24 or + linenum <= class_info.linenum): + return + + matched = Match(r'\s*(public|protected|private):', clean_lines.lines[linenum]) + if matched: + # Issue warning if the line before public/protected/private was + # not a blank line, but don't do this if the previous line contains + # "class" or "struct". This can happen two ways: + # - We are at the beginning of the class. + # - We are forward-declaring an inner class that is semantically + # private, but needed to be public for implementation reasons. + prev_line = clean_lines.lines[linenum - 1] + if (not IsBlankLine(prev_line) and + not Search(r'\b(class|struct)\b', prev_line)): + # Try a bit harder to find the beginning of the class. This is to + # account for multi-line base-specifier lists, e.g.: + # class Derived + # : public Base { + end_class_head = class_info.linenum + for i in range(class_info.linenum, linenum): + if Search(r'\{\s*$', clean_lines.lines[i]): + end_class_head = i + break + if end_class_head < linenum - 1: + error(filename, linenum, 'whitespace/blank_line', 3, + '"%s:" should be preceded by a blank line' % matched.group(1)) + + def GetPreviousNonBlankLine(clean_lines, linenum): """Return the most recent non-blank line and its line number. @@ -2039,17 +2144,18 @@ def GetLineWidth(line): """ if isinstance(line, unicode): width = 0 - for c in unicodedata.normalize('NFC', line): - if unicodedata.east_asian_width(c) in ('W', 'F'): + for uc in unicodedata.normalize('NFC', line): + if unicodedata.east_asian_width(uc) in ('W', 'F'): width += 2 - elif not unicodedata.combining(c): + elif not unicodedata.combining(uc): width += 1 return width else: return len(line) -def CheckStyle(filename, clean_lines, linenum, file_extension, error): +def CheckStyle(filename, clean_lines, linenum, file_extension, class_state, + error): """Checks rules from the 'C++ style rules' section of cppguide.html. Most of these rules are hard to test (naming, comment style), but we @@ -2119,8 +2225,12 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, error): # # URLs can be long too. It's possible to split these, but it makes them # harder to cut&paste. + # + # The "$Id:...$" comment may also get very long without it being the + # developers fault. if (not line.startswith('#include') and not is_header_guard and - not Match(r'^\s*//.*http(s?)://\S*$', line)): + not Match(r'^\s*//.*http(s?)://\S*$', line) and + not Match(r'^// \$Id:.*#[0-9]+ \$$', line)): line_width = GetLineWidth(line) if line_width > 100: error(filename, linenum, 'whitespace/line_length', 4, @@ -2145,6 +2255,9 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, error): CheckBraces(filename, clean_lines, linenum, error) CheckSpacing(filename, clean_lines, linenum, error) CheckCheck(filename, clean_lines, linenum, error) + if class_state and class_state.classinfo_stack: + CheckSectionSpacing(filename, clean_lines, + class_state.classinfo_stack[-1], linenum, error) _RE_PATTERN_INCLUDE_NEW_STYLE = re.compile(r'#include +"[^/]+\.h"') @@ -2330,6 +2443,63 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): error(filename, linenum, 'readability/streams', 3, 'Streams are highly discouraged.') + +def _GetTextInside(text, start_pattern): + """Retrieves all the text between matching open and close parentheses. + + Given a string of lines and a regular expression string, retrieve all the text + following the expression and between opening punctuation symbols like + (, [, or {, and the matching close-punctuation symbol. This properly nested + occurrences of the punctuations, so for the text like + printf(a(), b(c())); + a call to _GetTextInside(text, r'printf\(') will return 'a(), b(c())'. + start_pattern must match string having an open punctuation symbol at the end. + + Args: + text: The lines to extract text. Its comments and strings must be elided. + It can be single line and can span multiple lines. + start_pattern: The regexp string indicating where to start extracting + the text. + Returns: + 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 + # rewritten to use _GetTextInside (and use inferior regexp matching today). + + # Give opening punctuations to get the matching close-punctuations. + matching_punctuation = {'(': ')', '{': '}', '[': ']'} + closing_punctuation = set(matching_punctuation.itervalues()) + + # Find the position to start extracting text. + match = re.search(start_pattern, text, re.M) + if not match: # start_pattern not found in text. + return None + start_position = match.end(0) + + assert start_position > 0, ( + 'start_pattern must ends with an opening punctuation.') + assert text[start_position - 1] in matching_punctuation, ( + 'start_pattern must ends with an opening punctuation.') + # Stack of closing punctuations we expect to have in text after position. + punctuation_stack = [matching_punctuation[text[start_position - 1]]] + position = start_position + while punctuation_stack and position < len(text): + if text[position] == punctuation_stack[-1]: + punctuation_stack.pop() + elif text[position] in closing_punctuation: + # A closing punctuation without matching opening punctuations. + return None + elif text[position] in matching_punctuation: + punctuation_stack.append(matching_punctuation[text[position]]) + position += 1 + if punctuation_stack: + # Opening punctuations left without matching close-punctuations. + return None + # punctuations match. + return text[start_position:position - 1] + + def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, error): """Checks rules from the 'C++ language rules' section of cppguide.html. @@ -2375,7 +2545,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, # These are complicated re's. They try to capture the following: # paren (for fn-prototype start), typename, &, varname. For the const # version, we're willing for const to be before typename or after - # Don't check the implemention on same line. + # Don't check the implementation on same line. fnline = line.split('{', 1)[0] if (len(re.findall(r'\([^()]*\b(?:[\w:]|<[^()]*>)+(\s?&|&\s?)\w+', fnline)) > len(re.findall(r'\([^()]*\bconst\s+(?:typename\s+)?(?:struct\s+)?' @@ -2402,9 +2572,12 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, if match: # 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. + # 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. if (match.group(1) is None and # If new operator, then this isn't a cast - not Match(r'^\s*MOCK_(CONST_)?METHOD\d+(_T)?\(', line)): + not (Match(r'^\s*MOCK_(CONST_)?METHOD\d+(_T)?\(', line) or + Match(r'^\s*MockCallback<.*>', line))): error(filename, linenum, 'readability/casting', 4, 'Using deprecated casting style. ' 'Use static_cast<%s>(...) instead' % @@ -2412,11 +2585,19 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, 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". - CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], - 'reinterpret_cast', r'\((\w+\s?\*+\s?)\)', error) + 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 @@ -2515,11 +2696,19 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, # 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()) - match = re.search(r'\b((?:string)?printf)\s*\(([\w.\->()]+)\)', line, re.I) - if match: - error(filename, linenum, 'runtime/printf', 4, - 'Potential format string bug. Do %s("%%s", %s) instead.' - % (match.group(1), match.group(2))) + # TODO(sugawarayu): 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); + printf_args = _GetTextInside(line, r'(?i)\b(string)?printf\s*\(') + if printf_args: + match = Match(r'([\w.\->()]+)$', printf_args) + if match: + function_name = re.search(r'\b((?:string)?printf)\s*\(', + line, re.I).group(1) + error(filename, linenum, 'runtime/printf', 4, + 'Potential format string bug. Do %s("%%s", %s) instead.' + % (function_name, match.group(1))) # Check for potential memset bugs like memset(buf, sizeof(buf), 0). match = Search(r'memset\s*\(([^,]*),\s*([^,]*),\s*0\s*\)', line) @@ -2561,7 +2750,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, if Match(r'(.+::)?[A-Z][A-Z0-9_]*', tok): continue # A catch all for tricky sizeof cases, including 'sizeof expression', # 'sizeof(*type)', 'sizeof(const type)', 'sizeof(struct StructName)' - # requires skipping the next token becasue we split on ' ' and '*'. + # requires skipping the next token because we split on ' ' and '*'. if tok.startswith('sizeof'): skip_next = True continue @@ -2582,7 +2771,13 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, line) if match and linenum + 1 < clean_lines.NumLines(): next_line = clean_lines.elided[linenum + 1] - if not Search(r'^\s*};', next_line): + # We allow some, but not all, declarations of variables to be present + # in the statement that defines the class. The [\w\*,\s]* fragment of + # the regular expression below allows users to declare instances of + # the class or pointers to instances, but not less common types such + # as function pointers or arrays. It's a tradeoff between allowing + # reasonable code and avoiding trying to parse more C++ using regexps. + if not Search(r'^\s*}[\w\*,\s]*;', next_line): error(filename, linenum, 'readability/constructors', 3, match.group(1) + ' should be the last thing in the class') @@ -2610,20 +2805,24 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern, line: The line of code to check. raw_line: The raw line of code to check, with comments. cast_type: The string for the C++ cast to recommend. This is either - reinterpret_cast or static_cast, depending. + reinterpret_cast, static_cast, or const_cast, depending. pattern: The regular expression used to find C-style casts. error: The function to call with any errors found. + + Returns: + True if an error was emitted. + False otherwise. """ match = Search(pattern, line) if not match: - return + return False # e.g., sizeof(int) sizeof_match = Match(r'.*sizeof\s*$', line[0:match.start(1) - 1]) if sizeof_match: error(filename, linenum, 'runtime/sizeof', 1, 'Using sizeof(type). Use sizeof(varname) instead if possible') - return + return True remainder = line[match.end(0):] @@ -2634,24 +2833,28 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern, # eg, void foo(int); or void foo(int) const; # The equals check is for function pointer assignment. # eg, void *(*foo)(int) = ... + # The > is for MockCallback<...> ... # # Right now, this will only catch cases where there's a single argument, and # it's unnamed. It should probably be expanded to check for multiple # arguments with some unnamed. - function_match = Match(r'\s*(\)|=|(const)?\s*(;|\{|throw\(\)))', remainder) + function_match = Match(r'\s*(\)|=|(const)?\s*(;|\{|throw\(\)|>))', remainder) if function_match: if (not function_match.group(3) or function_match.group(3) == ';' or - raw_line.find('/*') < 0): + ('MockCallback<' not in raw_line and + '/*' not in raw_line)): error(filename, linenum, 'readability/function', 3, 'All parameters should be named in a function') - return + return True # At this point, all that should be left is actual casts. error(filename, linenum, 'readability/casting', 4, 'Using C-style cast. Use %s<%s>(...) instead' % (cast_type, match.group(1))) + return True + _HEADERS_CONTAINING_TEMPLATES = ( ('<deque>', ('deque',)), @@ -2690,11 +2893,6 @@ _HEADERS_CONTAINING_TEMPLATES = ( ('<slist>', ('slist',)), ) -_HEADERS_ACCEPTED_BUT_NOT_PROMOTED = { - # We can trust with reasonable confidence that map gives us pair<>, too. - 'pair<>': ('map', 'multimap', 'hash_map', 'hash_multimap') -} - _RE_PATTERN_STRING = re.compile(r'\bstring\b') _re_pattern_algorithm_header = [] @@ -2827,11 +3025,11 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, continue # String is special -- it is a non-templatized type in STL. - m = _RE_PATTERN_STRING.search(line) - if m: + matched = _RE_PATTERN_STRING.search(line) + if matched: # Don't warn about strings in non-STL namespaces: # (We check only the first match per line; good enough.) - prefix = line[:m.start()] + prefix = line[:matched.start()] if prefix.endswith('std::') or not prefix.endswith('::'): required['<string>'] = (linenum, 'string') @@ -2856,7 +3054,7 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, header_found = False # Use the absolute path so that matching works properly. - abs_filename = os.path.abspath(filename) + abs_filename = FileInfo(filename).FullName() # For Emacs's flymake. # If cpplint is invoked from Emacs's flymake, a temporary file is generated @@ -2869,7 +3067,8 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, # include_state is modified during iteration, so we iterate over a copy of # the keys. - for header in include_state.keys(): #NOLINT + header_keys = include_state.keys() + for header in header_keys: (same_module, common_path) = FilesBelongToSameModule(abs_filename, header) fullpath = common_path + header if same_module and UpdateIncludeState(fullpath, include_state, io): @@ -2886,19 +3085,40 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, # All the lines have been processed, report the errors found. for required_header_unstripped in required: template = required[required_header_unstripped][1] - if template in _HEADERS_ACCEPTED_BUT_NOT_PROMOTED: - headers = _HEADERS_ACCEPTED_BUT_NOT_PROMOTED[template] - if [True for header in headers if header in include_state]: - continue if required_header_unstripped.strip('<>"') not in include_state: error(filename, required[required_header_unstripped][0], 'build/include_what_you_use', 4, 'Add #include ' + required_header_unstripped + ' for ' + template) +_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 + specified explicitly, and such use isn't intended in any case. + + 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.raw_lines + line = raw[linenum] + match = _RE_PATTERN_EXPLICIT_MAKEPAIR.search(line) + if match: + error(filename, linenum, 'build/explicit_make_pair', + 4, # 4 = high confidence + 'Omit template arguments from make_pair OR use pair directly OR' + ' if appropriate, construct a pair directly') + + def ProcessLine(filename, file_extension, clean_lines, line, include_state, function_state, - class_state, error): + class_state, error, extra_check_functions=[]): """Processes a single line in the file. Args: @@ -2913,30 +3133,39 @@ def ProcessLine(filename, file_extension, the current stack of nested class declarations 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) CheckForFunctionLengths(filename, clean_lines, line, function_state, error) CheckForMultilineCommentsAndStrings(filename, clean_lines, line, error) - CheckStyle(filename, clean_lines, line, file_extension, error) + CheckStyle(filename, clean_lines, line, file_extension, class_state, error) CheckLanguage(filename, clean_lines, line, file_extension, include_state, error) CheckForNonStandardConstructs(filename, clean_lines, line, class_state, error) 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) - -def ProcessFileData(filename, file_extension, lines, error): +def ProcessFileData(filename, file_extension, lines, error, + extra_check_functions=[]): """Performs lint checks and reports any errors to the given error function. Args: filename: Filename of the file that is being processed. file_extension: The extension (dot not included) of the file. lines: An array of strings, each representing a line of the file, with the - last element being empty if the file is termined with a newline. + 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']) @@ -2956,7 +3185,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, class_state, error) + include_state, function_state, class_state, error, + extra_check_functions) class_state.CheckFinished(filename, error) CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error) @@ -2967,7 +3197,7 @@ def ProcessFileData(filename, file_extension, lines, error): CheckForNewlineAtEOF(filename, lines, error) -def ProcessFile(filename, vlevel): +def ProcessFile(filename, vlevel, extra_check_functions=[]): """Does google-lint on a single file. Args: @@ -2975,6 +3205,10 @@ def ProcessFile(filename, vlevel): 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) @@ -3019,9 +3253,10 @@ def ProcessFile(filename, vlevel): and file_extension != 'cpp'): sys.stderr.write('Ignoring %s; not a .cc or .h file\n' % filename) else: - ProcessFileData(filename, file_extension, lines, Error) + ProcessFileData(filename, file_extension, lines, Error, + extra_check_functions) if carriage_return_found and os.linesep != '\r\n': - # Use 0 for linenum since outputing only one error for potentially + # 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;' |