Index: cpplint.py |
=================================================================== |
--- cpplint.py (revision 100141) |
+++ cpplint.py (working copy) |
@@ -150,6 +150,7 @@ |
'build/class', |
'build/deprecated', |
'build/endif_comment', |
+ 'build/explicit_make_pair', |
'build/forward_decl', |
'build/header_guard', |
'build/include', |
@@ -210,11 +211,11 @@ |
# 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 @@ |
'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 @@ |
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 @@ |
_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 @@ |
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 @@ |
self.current_function, self.lines_in_function, trigger)) |
def End(self): |
- """Stop analizing function body.""" |
+ """Stop analyzing function body.""" |
self.in_a_function = False |
@@ -760,8 +761,7 @@ |
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 +913,7 @@ |
""" |
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 +966,7 @@ |
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: |
@@ -1248,7 +1248,7 @@ |
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,7 +1257,21 @@ |
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 +1392,16 @@ |
# 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 +1431,12 @@ |
# 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 +1527,14 @@ |
# 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 +1565,7 @@ |
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 +1661,8 @@ |
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 +1689,7 @@ |
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 +1740,11 @@ |
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 +1854,22 @@ |
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 +1901,58 @@ |
'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 +2130,18 @@ |
""" |
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 +2211,12 @@ |
# |
# 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 +2241,9 @@ |
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 +2429,63 @@ |
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 +2531,7 @@ |
# 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 +2558,12 @@ |
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,12 +2571,20 @@ |
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 |
# point where you think. |
@@ -2515,11 +2682,19 @@ |
# 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 +2736,7 @@ |
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 +2757,13 @@ |
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 +2791,24 @@ |
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,25 +2819,29 @@ |
# 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',)), |
('<functional>', ('unary_function', 'binary_function', |
@@ -2690,11 +2879,6 @@ |
('<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 +3011,11 @@ |
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') |
@@ -2869,7 +3053,8 @@ |
# 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 +3071,40 @@ |
# 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 +3119,39 @@ |
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 +3171,8 @@ |
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 +3183,7 @@ |
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 +3191,10 @@ |
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 +3239,10 @@ |
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;' |