StarMirror/styleguide
1
0
Fork 0
mirror of https://github.com/google/styleguide.git synced 2024-03-22 13:11:43 +08:00
Code Issues Projects Releases Wiki Activity

Update to #179:

Browse Source 
- Suggest "const_cast" when encountering "(char*)".
- Warn on template arguments to make_pair().
- Require <utility> if pair<> is seen now that <map> doesn't include it.
- Warn on lack of "explicit" keyword on single argument inline constructors.
- Better check for hanging ')' when closing function calls.
- Don't warn on: 'int v[1][3] = {{1, 2, 3}};'
- Allow function calls as the first argument to printf().
Review URL: http://codereview.appspot.com/4974066
This commit is contained in:
erg@google.com 2011-09-08 00:45:54 +00:00
parent efeacdf0fa
commit 8a95ecca27
2 changed files with 585 additions and 241 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

295
cpplint/cpplint.py vendored
View File

@ -150,6 +150,7 @@ _ERROR_CATEGORIES = [
'build/class', 'build/class',
'build/deprecated', 'build/deprecated',
'build/endif_comment', 'build/endif_comment',
'build/explicit_make_pair',
'build/forward_decl', 'build/forward_decl',
'build/header_guard', 'build/header_guard',
'build/include', 'build/include',
@ -210,11 +211,11 @@ _ERROR_CATEGORIES = [
# flag. By default all errors are on, so only add here categories that should be # 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). # 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. # 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 # 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 # 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. # Headers that we consider STL headers.
_STL_HEADERS = frozenset([ _STL_HEADERS = frozenset([
@ -310,9 +311,9 @@ def ParseNolintSuppressions(filename, raw_line, linenum, error):
error: function, an error handler. error: function, an error handler.
""" """
# FIXME(adonovan): "NOLINT(" is misparsed as NOLINT(*). # FIXME(adonovan): "NOLINT(" is misparsed as NOLINT(*).
m = _RE_SUPPRESSION.search(raw_line) matched = _RE_SUPPRESSION.search(raw_line)
if m: if matched:
category = m.group(1) category = matched.group(1)
if category in (None, '(*)'): # => "suppress all" if category in (None, '(*)'): # => "suppress all"
_error_suppressions.setdefault(None, set()).add(linenum) _error_suppressions.setdefault(None, set()).add(linenum)
else: else:
@ -404,7 +405,7 @@ class _IncludeState(dict):
self._last_header = '' self._last_header = ''
def CanonicalizeAlphabeticalOrder(self, header_path): 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. - replaces "-" with "_" so they both cmp the same.
- removes '-inl' since we don't require them to be after the main header. - 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)) self.current_function, self.lines_in_function, trigger))
def End(self): def End(self):
"""Stop analizing function body.""" """Stop analyzing function body."""
self.in_a_function = False self.in_a_function = False
@ -760,8 +761,7 @@ class FileInfo:
def _ShouldPrintError(category, confidence, linenum): def _ShouldPrintError(category, confidence, linenum):
"""Returns true iff confidence >= verbose, category passes """If confidence >= verbose, category passes filter and is not suppressed."""
filter and is not NOLINT-suppressed."""
# There are three ways we might decide not to print an error message: # There are three ways we might decide not to print an error message:
# a "NOLINT(category)" comment appears in the source, # a "NOLINT(category)" comment appears in the source,
@ -966,7 +966,7 @@ class CleansedLines(object):
def CloseExpression(clean_lines, linenum, pos): def CloseExpression(clean_lines, linenum, pos):
"""If input points to ( or { or [, finds the position that closes it. """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. linenum/pos that correspond to the closing of the expression.
Args: Args:
@ -1248,7 +1248,7 @@ def CheckInvalidIncrement(filename, clean_lines, linenum, error):
class _ClassInfo(object): class _ClassInfo(object):
"""Stores information about a class.""" """Stores information about a class."""
def __init__(self, name, linenum): def __init__(self, name, clean_lines, linenum):
self.name = name self.name = name
self.linenum = linenum self.linenum = linenum
self.seen_open_brace = False self.seen_open_brace = False
@ -1257,6 +1257,20 @@ class _ClassInfo(object):
self.has_virtual_destructor = False self.has_virtual_destructor = False
self.brace_depth = 0 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): class _ClassState(object):
"""Holds the current state of the parse relating to class declarations. """Holds the current state of the parse relating to class declarations.
@ -1383,9 +1397,11 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum,
# class LOCKABLE API Object { # class LOCKABLE API Object {
# }; # };
class_decl_match = Match( class_decl_match = Match(
r'\s*(template\s*<[\w\s<>,:]*>\s*)?(class|struct)\s+([A-Z_]+\s+)*(\w+(::\w+)*)', line) r'\s*(template\s*<[\w\s<>,:]*>\s*)?'
'(class|struct)\s+([A-Z_]+\s+)*(\w+(::\w+)*)', line)
if class_decl_match: if class_decl_match:
classinfo_stack.append(_ClassInfo(class_decl_match.group(4), 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 # Everything else in this function uses the top of the stack if it's
# not empty. # not empty.
@ -1415,12 +1431,12 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum,
# Look for single-argument constructors that aren't marked explicit. # Look for single-argument constructors that aren't marked explicit.
# Technically a valid construct, but against style. # 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), % re.escape(base_classname),
line) line)
if (args and if (args and
args.group(1) != 'void' 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())): args.group(1).strip())):
error(filename, linenum, 'runtime/explicit', 5, error(filename, linenum, 'runtime/explicit', 5,
'Single-argument constructors should be marked explicit.') 'Single-argument constructors should be marked explicit.')
@ -1511,6 +1527,12 @@ def CheckSpacingForFunctionCall(filename, line, linenum, error):
# If the ) is followed only by a newline or a { + newline, assume it's # 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 # part of a control statement (if/while/etc), and don't complain
if Search(r'[^)]\s+\)\s*[^{\s]', fncall): if Search(r'[^)]\s+\)\s*[^{\s]', fncall):
# 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, error(filename, linenum, 'whitespace/parens', 2,
'Extra space before )') 'Extra space before )')
@ -1543,7 +1565,7 @@ def CheckForFunctionLengths(filename, clean_lines, linenum,
Trivial bodies are unchecked, so constructors with huge initializer lists Trivial bodies are unchecked, so constructors with huge initializer lists
may be missed. may be missed.
Blank/comment lines are not counted so as to avoid encouraging the removal 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. NOLINT *on the last line of a function* disables this check.
Args: Args:
@ -1639,8 +1661,8 @@ def CheckSpacing(filename, clean_lines, linenum, error):
Things we check for: spaces around operators, spaces after Things we check for: spaces around operators, spaces after
if/for/while/switch, no spaces around parens in function calls, two 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 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 line, don't end a function with a blank line, don't add a blank line
blank lines in a row. after public/protected/private, don't have too many blank lines in a row.
Args: Args:
filename: The name of the current file. filename: The name of the current file.
@ -1667,7 +1689,7 @@ def CheckSpacing(filename, clean_lines, linenum, error):
and prev_line[:prevbrace].find('namespace') == -1): and prev_line[:prevbrace].find('namespace') == -1):
# OK, we have a blank line at the start of a code block. Before we # 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 # 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 # 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 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 # the previous line is indented 6 spaces, which may happen when the
@ -1718,6 +1740,11 @@ def CheckSpacing(filename, clean_lines, linenum, error):
error(filename, linenum, 'whitespace/blank_line', 3, error(filename, linenum, 'whitespace/blank_line', 3,
'Blank line at the end of a code block. Is this needed?') '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 # Next, we complain if there's a comment too near the text
commentpos = line.find('//') commentpos = line.find('//')
if commentpos != -1: if commentpos != -1:
@ -1838,10 +1865,11 @@ def CheckSpacing(filename, clean_lines, linenum, error):
# Next we will look for issues with function calls. # Next we will look for issues with function calls.
CheckSpacingForFunctionCall(filename, line, linenum, error) CheckSpacingForFunctionCall(filename, line, linenum, error)
# Except after an opening paren, you should have spaces before your braces. # Except after an opening paren, or after another opening brace (in case of
# And since you should never have braces at the beginning of a line, this is # an initializer list, for instance), you should have spaces before your
# an easy test. # braces. And since you should never have braces at the beginning of a line,
if Search(r'[^ (]{', line): # this is an easy test.
if Search(r'[^ ({]{', line):
error(filename, linenum, 'whitespace/braces', 5, error(filename, linenum, 'whitespace/braces', 5,
'Missing space before {') 'Missing space before {')
@ -1873,6 +1901,58 @@ def CheckSpacing(filename, clean_lines, linenum, error):
'statement, use { } instead.') '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): def GetPreviousNonBlankLine(clean_lines, linenum):
"""Return the most recent non-blank line and its line number. """Return the most recent non-blank line and its line number.
@ -2050,17 +2130,18 @@ def GetLineWidth(line):
""" """
if isinstance(line, unicode): if isinstance(line, unicode):
width = 0 width = 0
for c in unicodedata.normalize('NFC', line): for uc in unicodedata.normalize('NFC', line):
if unicodedata.east_asian_width(c) in ('W', 'F'): if unicodedata.east_asian_width(uc) in ('W', 'F'):
width += 2 width += 2
elif not unicodedata.combining(c): elif not unicodedata.combining(uc):
width += 1 width += 1
return width return width
else: else:
return len(line) 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. """Checks rules from the 'C++ style rules' section of cppguide.html.
Most of these rules are hard to test (naming, comment style), but we Most of these rules are hard to test (naming, comment style), but we
@ -2160,6 +2241,9 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, error):
CheckBraces(filename, clean_lines, linenum, error) CheckBraces(filename, clean_lines, linenum, error)
CheckSpacing(filename, clean_lines, linenum, error) CheckSpacing(filename, clean_lines, linenum, error)
CheckCheck(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"') _RE_PATTERN_INCLUDE_NEW_STYLE = re.compile(r'#include +"[^/]+\.h"')
@ -2345,6 +2429,63 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error):
error(filename, linenum, 'readability/streams', 3, error(filename, linenum, 'readability/streams', 3,
'Streams are highly discouraged.') '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, def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state,
error): error):
"""Checks rules from the 'C++ language rules' section of cppguide.html. """Checks rules from the 'C++ language rules' section of cppguide.html.
@ -2390,7 +2531,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state,
# These are complicated re's. They try to capture the following: # These are complicated re's. They try to capture the following:
# paren (for fn-prototype start), typename, &, varname. For the const # paren (for fn-prototype start), typename, &, varname. For the const
# version, we're willing for const to be before typename or after # 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] fnline = line.split('{', 1)[0]
if (len(re.findall(r'\([^()]*\b(?:[\w:]|<[^()]*>)+(\s?&|&\s?)\w+', fnline)) > if (len(re.findall(r'\([^()]*\b(?:[\w:]|<[^()]*>)+(\s?&|&\s?)\w+', fnline)) >
len(re.findall(r'\([^()]*\bconst\s+(?:typename\s+)?(?:struct\s+)?' len(re.findall(r'\([^()]*\bconst\s+(?:typename\s+)?(?:struct\s+)?'
@ -2430,9 +2571,17 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state,
CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum],
'static_cast', 'static_cast',
r'\((int|float|double|bool|char|u?int(16|32|64))\)', r'\((int|float|double|bool|char|u?int(16|32|64))\)', error)
error)
# This doesn't catch all cases. Consider (const char * const)"hello". # 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], CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum],
'reinterpret_cast', r'\((\w+\s?\*+\s?)\)', error) 'reinterpret_cast', r'\((\w+\s?\*+\s?)\)', error)
@ -2533,11 +2682,19 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state,
# Check for potential format string bugs like printf(foo). # Check for potential format string bugs like printf(foo).
# We constrain the pattern not to pick things like DocidForPrintf(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()) # 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) # 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: if match:
function_name = re.search(r'\b((?:string)?printf)\s*\(',
line, re.I).group(1)
error(filename, linenum, 'runtime/printf', 4, error(filename, linenum, 'runtime/printf', 4,
'Potential format string bug. Do %s("%%s", %s) instead.' 'Potential format string bug. Do %s("%%s", %s) instead.'
% (match.group(1), match.group(2))) % (function_name, match.group(1)))
# Check for potential memset bugs like memset(buf, sizeof(buf), 0). # Check for potential memset bugs like memset(buf, sizeof(buf), 0).
match = Search(r'memset\s*\(([^,]*),\s*([^,]*),\s*0\s*\)', line) match = Search(r'memset\s*\(([^,]*),\s*([^,]*),\s*0\s*\)', line)
@ -2579,7 +2736,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state,
if Match(r'(.+::)?[A-Z][A-Z0-9_]*', tok): continue if Match(r'(.+::)?[A-Z][A-Z0-9_]*', tok): continue
# A catch all for tricky sizeof cases, including 'sizeof expression', # A catch all for tricky sizeof cases, including 'sizeof expression',
# 'sizeof(*type)', 'sizeof(const type)', 'sizeof(struct StructName)' # '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'): if tok.startswith('sizeof'):
skip_next = True skip_next = True
continue continue
@ -2600,7 +2757,13 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state,
line) line)
if match and linenum + 1 < clean_lines.NumLines(): if match and linenum + 1 < clean_lines.NumLines():
next_line = clean_lines.elided[linenum + 1] 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, error(filename, linenum, 'readability/constructors', 3,
match.group(1) + ' should be the last thing in the class') match.group(1) + ' should be the last thing in the class')
@ -2628,20 +2791,24 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern,
line: The line of code to check. line: The line of code to check.
raw_line: The raw line of code to check, with comments. raw_line: The raw line of code to check, with comments.
cast_type: The string for the C++ cast to recommend. This is either 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. pattern: The regular expression used to find C-style casts.
error: The function to call with any errors found. error: The function to call with any errors found.
Returns:
True if an error was emitted.
False otherwise.
""" """
match = Search(pattern, line) match = Search(pattern, line)
if not match: if not match:
return return False
# e.g., sizeof(int) # e.g., sizeof(int)
sizeof_match = Match(r'.*sizeof\s*$', line[0:match.start(1) - 1]) sizeof_match = Match(r'.*sizeof\s*$', line[0:match.start(1) - 1])
if sizeof_match: if sizeof_match:
error(filename, linenum, 'runtime/sizeof', 1, error(filename, linenum, 'runtime/sizeof', 1,
'Using sizeof(type). Use sizeof(varname) instead if possible') 'Using sizeof(type). Use sizeof(varname) instead if possible')
return return True
remainder = line[match.end(0):] remainder = line[match.end(0):]
@ -2665,13 +2832,15 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern,
'/*' not in raw_line)): '/*' not in raw_line)):
error(filename, linenum, 'readability/function', 3, error(filename, linenum, 'readability/function', 3,
'All parameters should be named in a function') 'All parameters should be named in a function')
return return True
# At this point, all that should be left is actual casts. # At this point, all that should be left is actual casts.
error(filename, linenum, 'readability/casting', 4, error(filename, linenum, 'readability/casting', 4,
'Using C-style cast. Use %s<%s>(...) instead' % 'Using C-style cast. Use %s<%s>(...) instead' %
(cast_type, match.group(1))) (cast_type, match.group(1)))
return True
_HEADERS_CONTAINING_TEMPLATES = ( _HEADERS_CONTAINING_TEMPLATES = (
('<deque>', ('deque',)), ('<deque>', ('deque',)),
@ -2710,11 +2879,6 @@ _HEADERS_CONTAINING_TEMPLATES = (
('<slist>', ('slist',)), ('<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_STRING = re.compile(r'\bstring\b')
_re_pattern_algorithm_header = [] _re_pattern_algorithm_header = []
@ -2847,11 +3011,11 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error,
continue continue
# String is special -- it is a non-templatized type in STL. # String is special -- it is a non-templatized type in STL.
m = _RE_PATTERN_STRING.search(line) matched = _RE_PATTERN_STRING.search(line)
if m: if matched:
# Don't warn about strings in non-STL namespaces: # Don't warn about strings in non-STL namespaces:
# (We check only the first match per line; good enough.) # (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('::'): if prefix.endswith('std::') or not prefix.endswith('::'):
required['<string>'] = (linenum, 'string') required['<string>'] = (linenum, 'string')
@ -2889,7 +3053,8 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error,
# include_state is modified during iteration, so we iterate over a copy of # include_state is modified during iteration, so we iterate over a copy of
# the keys. # 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) (same_module, common_path) = FilesBelongToSameModule(abs_filename, header)
fullpath = common_path + header fullpath = common_path + header
if same_module and UpdateIncludeState(fullpath, include_state, io): if same_module and UpdateIncludeState(fullpath, include_state, io):
@ -2906,16 +3071,37 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error,
# All the lines have been processed, report the errors found. # All the lines have been processed, report the errors found.
for required_header_unstripped in required: for required_header_unstripped in required:
template = required[required_header_unstripped][1] 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: if required_header_unstripped.strip('<>"') not in include_state:
error(filename, required[required_header_unstripped][0], error(filename, required[required_header_unstripped][0],
'build/include_what_you_use', 4, 'build/include_what_you_use', 4,
'Add #include ' + required_header_unstripped + ' for ' + template) '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, def ProcessLine(filename, file_extension,
clean_lines, line, include_state, function_state, clean_lines, line, include_state, function_state,
class_state, error, extra_check_functions=[]): class_state, error, extra_check_functions=[]):
@ -2941,13 +3127,14 @@ def ProcessLine(filename, file_extension,
ParseNolintSuppressions(filename, raw_lines[line], line, error) ParseNolintSuppressions(filename, raw_lines[line], line, error)
CheckForFunctionLengths(filename, clean_lines, line, function_state, error) CheckForFunctionLengths(filename, clean_lines, line, function_state, error)
CheckForMultilineCommentsAndStrings(filename, clean_lines, line, 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, CheckLanguage(filename, clean_lines, line, file_extension, include_state,
error) error)
CheckForNonStandardConstructs(filename, clean_lines, line, CheckForNonStandardConstructs(filename, clean_lines, line,
class_state, error) class_state, error)
CheckPosixThreading(filename, clean_lines, line, error) CheckPosixThreading(filename, clean_lines, line, error)
CheckInvalidIncrement(filename, clean_lines, line, error) CheckInvalidIncrement(filename, clean_lines, line, error)
CheckMakePairUsesDeduction(filename, clean_lines, line, error)
for check_fn in extra_check_functions: for check_fn in extra_check_functions:
check_fn(filename, clean_lines, line, error) check_fn(filename, clean_lines, line, error)
@ -2959,7 +3146,7 @@ def ProcessFileData(filename, file_extension, lines, error,
filename: Filename of the file that is being processed. filename: Filename of the file that is being processed.
file_extension: The extension (dot not included) of the file. file_extension: The extension (dot not included) of the file.
lines: An array of strings, each representing a line of the file, with the 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: error: A callable to which errors are reported, which takes 4 arguments:
filename, line number, error level, and message filename, line number, error level, and message
extra_check_functions: An array of additional check functions that will be extra_check_functions: An array of additional check functions that will be
@ -3055,7 +3242,7 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]):
ProcessFileData(filename, file_extension, lines, Error, ProcessFileData(filename, file_extension, lines, Error,
extra_check_functions) extra_check_functions)
if carriage_return_found and os.linesep != '\r\n': 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. # several lines.
Error(filename, 0, 'whitespace/newline', 1, Error(filename, 0, 'whitespace/newline', 1,
'One or more unexpected \\r (^M) found;' 'One or more unexpected \\r (^M) found;'

497
cpplint/cpplint_unittest.py
View File

@ -134,7 +134,8 @@ class CpplintTestBase(unittest.TestCase):
lines = cpplint.CleansedLines(lines) lines = cpplint.CleansedLines(lines)
class_state = cpplint._ClassState() class_state = cpplint._ClassState()
for i in xrange(lines.NumLines()): for i in xrange(lines.NumLines()):
cpplint.CheckStyle('foo.h', lines, i, 'h', error_collector) cpplint.CheckStyle('foo.h', lines, i, 'h', class_state,
error_collector)
cpplint.CheckForNonStandardConstructs('foo.h', lines, i, class_state, cpplint.CheckForNonStandardConstructs('foo.h', lines, i, class_state,
error_collector) error_collector)
class_state.CheckFinished('foo.h', error_collector) class_state.CheckFinished('foo.h', error_collector)
@ -243,6 +244,29 @@ class CpplintTest(CpplintTestBase):
self.assertEquals(10, cpplint.GetLineWidth(u'x' * 10)) self.assertEquals(10, cpplint.GetLineWidth(u'x' * 10))
self.assertEquals(16, cpplint.GetLineWidth(u'都|道|府|県|支庁')) self.assertEquals(16, cpplint.GetLineWidth(u'都|道|府|県|支庁'))
def testGetTextInside(self):
self.assertEquals('', cpplint._GetTextInside('fun()', r'fun\('))
self.assertEquals('x, y', cpplint._GetTextInside('f(x, y)', r'f\('))
self.assertEquals('a(), b(c())', cpplint._GetTextInside(
'printf(a(), b(c()))', r'printf\('))
self.assertEquals('x, y{}', cpplint._GetTextInside('f[x, y{}]', r'f\['))
self.assertEquals(None, cpplint._GetTextInside('f[a, b(}]', r'f\['))
self.assertEquals(None, cpplint._GetTextInside('f[x, y]', r'f\('))
self.assertEquals('y, h(z, (a + b))', cpplint._GetTextInside(
'f(x, g(y, h(z, (a + b))))', r'g\('))
self.assertEquals('f(f(x))', cpplint._GetTextInside('f(f(f(x)))', r'f\('))
# Supports multiple lines.
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.
self.assertEquals('x, y',
cpplint._GetTextInside(
'#include "inl.h" // skip #define\n'
'#define A2(x, y) a_inl_(x, y, __LINE__)\n'
'#define A(x) a_inl_(x, "", __LINE__)\n',
r'^\s*#define\s*\w+\('))
def testFindNextMultiLineCommentStart(self): def testFindNextMultiLineCommentStart(self):
self.assertEquals(1, cpplint.FindNextMultiLineCommentStart([''], 0)) self.assertEquals(1, cpplint.FindNextMultiLineCommentStart([''], 0))
@ -467,6 +491,19 @@ class CpplintTest(CpplintTestBase):
'Using deprecated casting style. ' 'Using deprecated casting style. '
'Use static_cast<int>(...) instead' 'Use static_cast<int>(...) instead'
' [readability/casting] [4]') ' [readability/casting] [4]')
self.TestLint(
'(char *) "foo"',
'Using C-style cast. '
'Use const_cast<char *>(...) instead'
' [readability/casting] [4]')
self.TestLint(
'(int*)foo',
'Using C-style cast. '
'Use reinterpret_cast<int*>(...) instead'
' [readability/casting] [4]')
# Checks for false positives... # Checks for false positives...
self.TestLint( self.TestLint(
'int a = int(); // Constructor, o.k.', 'int a = int(); // Constructor, o.k.',
@ -554,74 +591,77 @@ class CpplintTest(CpplintTestBase):
def testIncludeWhatYouUse(self): def testIncludeWhatYouUse(self):
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include <vector> """#include <vector>
std::vector<int> foo; std::vector<int> foo;
''', """,
'') '')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include <map> """#include <map>
std::pair<int,int> foo; std::pair<int,int> foo;
''', """,
'')
self.TestIncludeWhatYouUse(
'''#include <multimap>
std::pair<int,int> foo;
''',
'')
self.TestIncludeWhatYouUse(
'''#include <hash_map>
std::pair<int,int> foo;
''',
'')
self.TestIncludeWhatYouUse(
'''#include <utility>
std::pair<int,int> foo;
''',
'')
self.TestIncludeWhatYouUse(
'''#include <vector>
DECLARE_string(foobar);
''',
'')
self.TestIncludeWhatYouUse(
'''#include <vector>
DEFINE_string(foobar, "", "");
''',
'')
self.TestIncludeWhatYouUse(
'''#include <vector>
std::pair<int,int> foo;
''',
'Add #include <utility> for pair<>' 'Add #include <utility> for pair<>'
' [build/include_what_you_use] [4]') ' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include "base/foobar.h" """#include <multimap>
std::pair<int,int> foo;
""",
'Add #include <utility> for pair<>'
' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse(
"""#include <hash_map>
std::pair<int,int> foo;
""",
'Add #include <utility> for pair<>'
' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse(
"""#include <utility>
std::pair<int,int> foo;
""",
'')
self.TestIncludeWhatYouUse(
"""#include <vector>
DECLARE_string(foobar);
""",
'')
self.TestIncludeWhatYouUse(
"""#include <vector>
DEFINE_string(foobar, "", "");
""",
'')
self.TestIncludeWhatYouUse(
"""#include <vector>
std::pair<int,int> foo;
""",
'Add #include <utility> for pair<>'
' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse(
"""#include "base/foobar.h"
std::vector<int> foo; std::vector<int> foo;
''', """,
'Add #include <vector> for vector<>' 'Add #include <vector> for vector<>'
' [build/include_what_you_use] [4]') ' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include <vector> """#include <vector>
std::set<int> foo; std::set<int> foo;
''', """,
'Add #include <set> for set<>' 'Add #include <set> for set<>'
' [build/include_what_you_use] [4]') ' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include "base/foobar.h" """#include "base/foobar.h"
hash_map<int, int> foobar; hash_map<int, int> foobar;
''', """,
'Add #include <hash_map> for hash_map<>' 'Add #include <hash_map> for hash_map<>'
' [build/include_what_you_use] [4]') ' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include "base/foobar.h" """#include "base/foobar.h"
bool foobar = std::less<int>(0,1); bool foobar = std::less<int>(0,1);
''', """,
'Add #include <functional> for less<>' 'Add #include <functional> for less<>'
' [build/include_what_you_use] [4]') ' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include "base/foobar.h" """#include "base/foobar.h"
bool foobar = min<int>(0,1); bool foobar = min<int>(0,1);
''', """,
'Add #include <algorithm> for min [build/include_what_you_use] [4]') 'Add #include <algorithm> for min [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'void a(const string &foobar);', 'void a(const string &foobar);',
@ -633,55 +673,55 @@ class CpplintTest(CpplintTestBase):
'void a(const my::string &foobar);', 'void a(const my::string &foobar);',
'') # Avoid false positives on strings in other namespaces. '') # Avoid false positives on strings in other namespaces.
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include "base/foobar.h" """#include "base/foobar.h"
bool foobar = swap(0,1); bool foobar = swap(0,1);
''', """,
'Add #include <algorithm> for swap [build/include_what_you_use] [4]') 'Add #include <algorithm> for swap [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include "base/foobar.h" """#include "base/foobar.h"
bool foobar = transform(a.begin(), a.end(), b.start(), Foo); bool foobar = transform(a.begin(), a.end(), b.start(), Foo);
''', """,
'Add #include <algorithm> for transform ' 'Add #include <algorithm> for transform '
'[build/include_what_you_use] [4]') '[build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include "base/foobar.h" """#include "base/foobar.h"
bool foobar = min_element(a.begin(), a.end()); bool foobar = min_element(a.begin(), a.end());
''', """,
'Add #include <algorithm> for min_element ' 'Add #include <algorithm> for min_element '
'[build/include_what_you_use] [4]') '[build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''foo->swap(0,1); """foo->swap(0,1);
foo.swap(0,1); foo.swap(0,1);
''', """,
'') '')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include <string> """#include <string>
void a(const std::multimap<int,string> &foobar); void a(const std::multimap<int,string> &foobar);
''', """,
'Add #include <map> for multimap<>' 'Add #include <map> for multimap<>'
' [build/include_what_you_use] [4]') ' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include <queue> """#include <queue>
void a(const std::priority_queue<int> &foobar); void a(const std::priority_queue<int> &foobar);
''', """,
'') '')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include <assert.h> """#include <assert.h>
#include <string> #include <string>
#include <vector> #include <vector>
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/port.h" #include "base/port.h"
vector<string> hajoa;''', '') vector<string> hajoa;""", '')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include <string> """#include <string>
int i = numeric_limits<int>::max() int i = numeric_limits<int>::max()
''', """,
'Add #include <limits> for numeric_limits<>' 'Add #include <limits> for numeric_limits<>'
' [build/include_what_you_use] [4]') ' [build/include_what_you_use] [4]')
self.TestIncludeWhatYouUse( self.TestIncludeWhatYouUse(
'''#include <limits> """#include <limits>
int i = numeric_limits<int>::max() int i = numeric_limits<int>::max()
''', """,
'') '')
# Test the UpdateIncludeState code path. # Test the UpdateIncludeState code path.
@ -694,8 +734,8 @@ class CpplintTest(CpplintTestBase):
mock_header_contents = ['#include <set>'] mock_header_contents = ['#include <set>']
message = self.PerformIncludeWhatYouUse( message = self.PerformIncludeWhatYouUse(
'''#include "blah/a.h" """#include "blah/a.h"
std::set<int> foo;''', std::set<int> foo;""",
filename='blah/a.cc', filename='blah/a.cc',
io=MockIo(mock_header_contents)) io=MockIo(mock_header_contents))
self.assertEquals(message, '') self.assertEquals(message, '')
@ -704,8 +744,8 @@ class CpplintTest(CpplintTestBase):
# a temporary file generated by Emacs's flymake. # a temporary file generated by Emacs's flymake.
mock_header_contents = [''] mock_header_contents = ['']
message = self.PerformIncludeWhatYouUse( message = self.PerformIncludeWhatYouUse(
'''#include "blah/a.h" """#include "blah/a.h"
std::set<int> foo;''', std::set<int> foo;""",
filename='blah/a_flymake.cc', filename='blah/a_flymake.cc',
io=MockIo(mock_header_contents)) io=MockIo(mock_header_contents))
self.assertEquals(message, 'Add #include <set> for set<> ' self.assertEquals(message, 'Add #include <set> for set<> '
@ -713,16 +753,16 @@ class CpplintTest(CpplintTestBase):
# If there's just a cc and the header can't be found then it's ok. # If there's just a cc and the header can't be found then it's ok.
message = self.PerformIncludeWhatYouUse( message = self.PerformIncludeWhatYouUse(
'''#include "blah/a.h" """#include "blah/a.h"
std::set<int> foo;''', std::set<int> foo;""",
filename='blah/a.cc') filename='blah/a.cc')
self.assertEquals(message, '') self.assertEquals(message, '')
# Make sure we find the headers with relative paths. # Make sure we find the headers with relative paths.
mock_header_contents = [''] mock_header_contents = ['']
message = self.PerformIncludeWhatYouUse( message = self.PerformIncludeWhatYouUse(
'''#include "%s/a.h" """#include "%s/a.h"
std::set<int> foo;''' % os.path.basename(os.getcwd()), std::set<int> foo;""" % os.path.basename(os.getcwd()),
filename='a.cc', filename='a.cc',
io=MockIo(mock_header_contents)) io=MockIo(mock_header_contents))
self.assertEquals(message, 'Add #include <set> for set<> ' self.assertEquals(message, 'Add #include <set> for set<> '
@ -770,22 +810,22 @@ class CpplintTest(CpplintTestBase):
def testMultiLineComments(self): def testMultiLineComments(self):
# missing explicit is bad # missing explicit is bad
self.TestMultiLineLint( self.TestMultiLineLint(
r'''int a = 0; r"""int a = 0;
/* multi-liner /* multi-liner
class Foo { class Foo {
Foo(int f); // should cause a lint warning in code Foo(int f); // should cause a lint warning in code
} }
*/ ''', */ """,
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
r'''/* int a = 0; multi-liner r"""/* int a = 0; multi-liner
static const int b = 0;''', static const int b = 0;""",
'Could not find end of multi-line comment' 'Could not find end of multi-line comment'
' [readability/multiline_comment] [5]') ' [readability/multiline_comment] [5]')
self.TestMultiLineLint(r''' /* multi-line comment''', self.TestMultiLineLint(r""" /* multi-line comment""",
'Could not find end of multi-line comment' 'Could not find end of multi-line comment'
' [readability/multiline_comment] [5]') ' [readability/multiline_comment] [5]')
self.TestMultiLineLint(r''' // /* comment, but not multi-line''', '') self.TestMultiLineLint(r""" // /* comment, but not multi-line""", '')
def testMultilineStrings(self): def testMultilineStrings(self):
multiline_string_error_message = ( multiline_string_error_message = (
@ -809,131 +849,151 @@ class CpplintTest(CpplintTestBase):
def testExplicitSingleArgumentConstructors(self): def testExplicitSingleArgumentConstructors(self):
# missing explicit is bad # missing explicit is bad
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Foo(int f); Foo(int f);
};''', };""",
'Single-argument constructors should be marked explicit.' 'Single-argument constructors should be marked explicit.'
' [runtime/explicit] [5]') ' [runtime/explicit] [5]')
# missing explicit is bad, even with whitespace # missing explicit is bad, even with whitespace
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Foo (int f); Foo (int f);
};''', };""",
['Extra space before ( in function call [whitespace/parens] [4]', ['Extra space before ( in function call [whitespace/parens] [4]',
'Single-argument constructors should be marked explicit.' 'Single-argument constructors should be marked explicit.'
' [runtime/explicit] [5]']) ' [runtime/explicit] [5]'])
# missing explicit, with distracting comment, is still bad # missing explicit, with distracting comment, is still bad
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Foo(int f); // simpler than Foo(blargh, blarg) Foo(int f); // simpler than Foo(blargh, blarg)
};''', };""",
'Single-argument constructors should be marked explicit.' 'Single-argument constructors should be marked explicit.'
' [runtime/explicit] [5]') ' [runtime/explicit] [5]')
# missing explicit, with qualified classname # missing explicit, with qualified classname
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Qualifier::AnotherOne::Foo { """class Qualifier::AnotherOne::Foo {
Foo(int f); Foo(int f);
};''', };""",
'Single-argument constructors should be marked explicit.'
' [runtime/explicit] [5]')
# missing explicit for inline constructors is bad as well
self.TestMultiLineLint(
"""class Foo {
inline Foo(int f);
};""",
'Single-argument constructors should be marked explicit.' 'Single-argument constructors should be marked explicit.'
' [runtime/explicit] [5]') ' [runtime/explicit] [5]')
# structs are caught as well. # structs are caught as well.
self.TestMultiLineLint( self.TestMultiLineLint(
'''struct Foo { """struct Foo {
Foo(int f); Foo(int f);
};''', };""",
'Single-argument constructors should be marked explicit.' 'Single-argument constructors should be marked explicit.'
' [runtime/explicit] [5]') ' [runtime/explicit] [5]')
# Templatized classes are caught as well. # Templatized classes are caught as well.
self.TestMultiLineLint( self.TestMultiLineLint(
'''template<typename T> class Foo { """template<typename T> class Foo {
Foo(int f); Foo(int f);
};''', };""",
'Single-argument constructors should be marked explicit.'
' [runtime/explicit] [5]')
# inline case for templatized classes.
self.TestMultiLineLint(
"""template<typename T> class Foo {
inline Foo(int f);
};""",
'Single-argument constructors should be marked explicit.' 'Single-argument constructors should be marked explicit.'
' [runtime/explicit] [5]') ' [runtime/explicit] [5]')
# proper style is okay # proper style is okay
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
explicit Foo(int f); explicit Foo(int f);
};''', };""",
'') '')
# two argument constructor is okay # two argument constructor is okay
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Foo(int f, int b); Foo(int f, int b);
};''', };""",
'') '')
# two argument constructor, across two lines, is okay # two argument constructor, across two lines, is okay
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Foo(int f, Foo(int f,
int b); int b);
};''', };""",
'') '')
# non-constructor (but similar name), is okay # non-constructor (but similar name), is okay
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
aFoo(int f); aFoo(int f);
};''', };""",
'') '')
# constructor with void argument is okay # constructor with void argument is okay
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Foo(void); Foo(void);
};''', };""",
'') '')
# single argument method is okay # single argument method is okay
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Bar(int b); Bar(int b);
};''', };""",
'') '')
# comments should be ignored # comments should be ignored
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
// Foo(int f); // Foo(int f);
};''', };""",
'') '')
# single argument function following class definition is okay # single argument function following class definition is okay
# (okay, it's not actually valid, but we don't want a false positive) # (okay, it's not actually valid, but we don't want a false positive)
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Foo(int f, int b); Foo(int f, int b);
}; };
Foo(int f);''', Foo(int f);""",
'') '')
# single argument function is okay # single argument function is okay
self.TestMultiLineLint( self.TestMultiLineLint(
'''static Foo(int f);''', """static Foo(int f);""",
'') '')
# single argument copy constructor is okay. # single argument copy constructor is okay.
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Foo(const Foo&); Foo(const Foo&);
};''', };""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
Foo(Foo&); Foo(Foo&);
};''', };""",
'')
# templatized copy constructor is okay.
self.TestMultiLineLint(
"""template<typename T> class Foo {
Foo(const Foo<T>&);
};""",
'') '')
def testSlashStarCommentOnSingleLine(self): def testSlashStarCommentOnSingleLine(self):
self.TestMultiLineLint( self.TestMultiLineLint(
'''/* static */ Foo(int f);''', """/* static */ Foo(int f);""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'''/*/ static */ Foo(int f);''', """/*/ static */ Foo(int f);""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'''/*/ static Foo(int f);''', """/*/ static Foo(int f);""",
'Could not find end of multi-line comment' 'Could not find end of multi-line comment'
' [readability/multiline_comment] [5]') ' [readability/multiline_comment] [5]')
self.TestMultiLineLint( self.TestMultiLineLint(
''' /*/ static Foo(int f);''', """ /*/ static Foo(int f);""",
'Could not find end of multi-line comment' 'Could not find end of multi-line comment'
' [readability/multiline_comment] [5]') ' [readability/multiline_comment] [5]')
self.TestMultiLineLint( self.TestMultiLineLint(
''' /**/ static Foo(int f);''', """ /**/ static Foo(int f);""",
'') '')
# Test suspicious usage of "if" like this: # Test suspicious usage of "if" like this:
@ -1017,6 +1077,9 @@ class CpplintTest(CpplintTestBase):
self.TestLint('printf("foo")', '') self.TestLint('printf("foo")', '')
self.TestLint('printf("foo: %s", foo)', '') self.TestLint('printf("foo: %s", foo)', '')
self.TestLint('DocidForPrintf(docid)', '') # Should not trigger. self.TestLint('DocidForPrintf(docid)', '') # Should not trigger.
self.TestLint('printf(format, value)', '') # Should not trigger.
self.TestLint('printf(format.c_str(), value)', '') # Should not trigger.
self.TestLint('printf(format(index).c_str(), value)', '')
self.TestLint( self.TestLint(
'printf(foo)', 'printf(foo)',
'Potential format string bug. Do printf("%s", foo) instead.' 'Potential format string bug. Do printf("%s", foo) instead.'
@ -1121,15 +1184,27 @@ class CpplintTest(CpplintTestBase):
'DISALLOW_IMPLICIT_CONSTRUCTORS'): 'DISALLOW_IMPLICIT_CONSTRUCTORS'):
self.TestLanguageRulesCheck( self.TestLanguageRulesCheck(
'some_class.h', 'some_class.h',
'''%s(SomeClass); """%s(SomeClass);
int foo_; int foo_;
};''' % macro_name, };""" % macro_name,
('%s should be the last thing in the class' % macro_name) + ('%s should be the last thing in the class' % macro_name) +
' [readability/constructors] [3]') ' [readability/constructors] [3]')
self.TestLanguageRulesCheck( self.TestLanguageRulesCheck(
'some_class.h', 'some_class.h',
'''%s(SomeClass); """%s(SomeClass);
};''' % macro_name, };""" % macro_name,
'')
self.TestLanguageRulesCheck(
'some_class.h',
"""%s(SomeClass);
int foo_;
} instance, *pointer_to_instance;""" % macro_name,
('%s should be the last thing in the class' % macro_name) +
' [readability/constructors] [3]')
self.TestLanguageRulesCheck(
'some_class.h',
"""%s(SomeClass);
} instance, *pointer_to_instance;""" % macro_name,
'') '')
# Brace usage # Brace usage
@ -1138,23 +1213,23 @@ class CpplintTest(CpplintTestBase):
# or initializing an array # or initializing an array
self.TestLint('int a[3] = { 1, 2, 3 };', '') self.TestLint('int a[3] = { 1, 2, 3 };', '')
self.TestLint( self.TestLint(
'''const int foo[] = """const int foo[] =
{1, 2, 3 };''', {1, 2, 3 };""",
'') '')
# For single line, unmatched '}' with a ';' is ignored (not enough context) # For single line, unmatched '}' with a ';' is ignored (not enough context)
self.TestMultiLineLint( self.TestMultiLineLint(
'''int a[3] = { 1, """int a[3] = { 1,
2, 2,
3 };''', 3 };""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'''int a[2][3] = { { 1, 2 }, """int a[2][3] = { { 1, 2 },
{ 3, 4 } };''', { 3, 4 } };""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'''int a[2][3] = """int a[2][3] =
{ { 1, 2 }, { { 1, 2 },
{ 3, 4 } };''', { 3, 4 } };""",
'') '')
# CHECK/EXPECT_TRUE/EXPECT_FALSE replacements # CHECK/EXPECT_TRUE/EXPECT_FALSE replacements
@ -1394,6 +1469,10 @@ class CpplintTest(CpplintTestBase):
self.TestLint('} else {', '') self.TestLint('} else {', '')
self.TestLint('} else if', '') self.TestLint('} else if', '')
def testSpacingWithInitializerLists(self):
self.TestLint('int v[1][3] = {{1, 2, 3}};', '')
self.TestLint('int v[1][1] = {{0}};', '')
def testSpacingForBinaryOps(self): def testSpacingForBinaryOps(self):
self.TestLint('if (foo<=bar) {', 'Missing spaces around <=' self.TestLint('if (foo<=bar) {', 'Missing spaces around <='
' [whitespace/operators] [3]') ' [whitespace/operators] [3]')
@ -1483,6 +1562,19 @@ class CpplintTest(CpplintTestBase):
' [whitespace/parens] [2]') ' [whitespace/parens] [2]')
self.TestLint('TellStory(1 /* wolf */, 3 /* pigs */);', self.TestLint('TellStory(1 /* wolf */, 3 /* pigs */);',
'') '')
self.TestMultiLineLint("""TellStory(1, 3
);""",
'Closing ) should be moved to the previous line'
' [whitespace/parens] [2]')
self.TestMultiLineLint("""TellStory(Wolves(1),
Pigs(3
));""",
'Closing ) should be moved to the previous line'
' [whitespace/parens] [2]')
self.TestMultiLineLint("""TellStory(1,
3 );""",
'Extra space before )'
' [whitespace/parens] [2]')
def testToDoComments(self): def testToDoComments(self):
start_space = ('Too many spaces before TODO' start_space = ('Too many spaces before TODO'
@ -1648,6 +1740,59 @@ class CpplintTest(CpplintTestBase):
'Blank line at the end of a code block. Is this needed?' 'Blank line at the end of a code block. Is this needed?'
' [whitespace/blank_line] [3]')) ' [whitespace/blank_line] [3]'))
def testBlankLineBeforeSectionKeyword(self):
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData('foo.cc', 'cc',
['class A {',
' public:',
' protected:', # warning 1
' private:', # warning 2
' struct B {',
' public:',
' private:'] + # warning 3
([''] * 100) + # Make A and B longer than 100 lines
[' };',
' struct C {',
' protected:',
' private:', # C is too short for warnings
' };',
'};',
'class D',
' : public {',
' public:', # no warning
'};'],
error_collector)
self.assertEquals(2, error_collector.Results().count(
'"private:" should be preceded by a blank line'
' [whitespace/blank_line] [3]'))
self.assertEquals(1, error_collector.Results().count(
'"protected:" should be preceded by a blank line'
' [whitespace/blank_line] [3]'))
def testNoBlankLineAfterSectionKeyword(self):
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData('foo.cc', 'cc',
['class A {',
' public:',
'', # warning 1
' private:',
'', # warning 2
' struct B {',
' protected:',
'', # warning 3
' };',
'};'],
error_collector)
self.assertEquals(1, error_collector.Results().count(
'Do not leave a blank line after "public:"'
' [whitespace/blank_line] [3]'))
self.assertEquals(1, error_collector.Results().count(
'Do not leave a blank line after "protected:"'
' [whitespace/blank_line] [3]'))
self.assertEquals(1, error_collector.Results().count(
'Do not leave a blank line after "private:"'
' [whitespace/blank_line] [3]'))
def testElseOnSameLineAsClosingBraces(self): def testElseOnSameLineAsClosingBraces(self):
error_collector = ErrorCollector(self.assert_) error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData('foo.cc', 'cc', cpplint.ProcessFileData('foo.cc', 'cc',
@ -1796,7 +1941,7 @@ class CpplintTest(CpplintTestBase):
def testDefaultFilter(self): def testDefaultFilter(self):
default_filters = cpplint._DEFAULT_FILTERS default_filters = cpplint._DEFAULT_FILTERS
old_filters = cpplint._cpplint_state.filters old_filters = cpplint._cpplint_state.filters
cpplint._DEFAULT_FILTERS = [ '-whitespace' ] cpplint._DEFAULT_FILTERS = ['-whitespace']
try: try:
# Reset filters # Reset filters
cpplint._cpplint_state.SetFilters('') cpplint._cpplint_state.SetFilters('')
@ -1838,19 +1983,19 @@ class CpplintTest(CpplintTestBase):
'class Foo;', 'class Foo;',
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'''struct Foo* """struct Foo*
foo = NewFoo();''', foo = NewFoo();""",
'') '')
# Here is an example where the linter gets confused, even though # Here is an example where the linter gets confused, even though
# the code doesn't violate the style guide. # the code doesn't violate the style guide.
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo """class Foo
#ifdef DERIVE_FROM_GOO #ifdef DERIVE_FROM_GOO
: public Goo { : public Goo {
#else #else
: public Hoo { : public Hoo {
#endif #endif
};''', };""",
'Failed to find complete declaration of class Foo' 'Failed to find complete declaration of class Foo'
' [build/class] [5]') ' [build/class] [5]')
@ -2117,7 +2262,7 @@ class CpplintTest(CpplintTestBase):
' [runtime/printf_format] [3]') ' [runtime/printf_format] [3]')
self.TestLint( self.TestLint(
r'snprintf(file, "Never mix %d and %1$d parmaeters!", value);', r'snprintf(file, "Never mix %d and %1$d parameters!", value);',
'%N$ formats are unconventional. Try rewriting to avoid them.' '%N$ formats are unconventional. Try rewriting to avoid them.'
' [runtime/printf_format] [2]') ' [runtime/printf_format] [2]')
@ -2774,48 +2919,48 @@ class NoNonVirtualDestructorsTest(CpplintTestBase):
def testNoError(self): def testNoError(self):
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
virtual ~Foo(); virtual ~Foo();
virtual void foo(); virtual void foo();
};''', };""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
virtual inline ~Foo(); virtual inline ~Foo();
virtual void foo(); virtual void foo();
};''', };""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo { """class Foo {
inline virtual ~Foo(); inline virtual ~Foo();
virtual void foo(); virtual void foo();
};''', };""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo::Goo { """class Foo::Goo {
virtual ~Goo(); virtual ~Goo();
virtual void goo(); virtual void goo();
};''', };""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
'class Foo { void foo(); };', 'class Foo { void foo(); };',
'More than one command on the same line [whitespace/newline] [4]') 'More than one command on the same line [whitespace/newline] [4]')
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Qualified::Goo : public Foo { """class Qualified::Goo : public Foo {
virtual void goo(); virtual void goo();
};''', };""",
'') '')
self.TestMultiLineLint( self.TestMultiLineLint(
# Line-ending : # Line-ending :
'''class Goo : """class Goo :
public Foo { public Foo {
virtual void goo(); virtual void goo();
};''', };""",
'Labels should always be indented at least one space. ' 'Labels should always be indented at least one space. '
'If this is a member-initializer list in a constructor or ' 'If this is a member-initializer list in a constructor or '
'the base class list in a class definition, the colon should ' 'the base class list in a class definition, the colon should '
@ -2823,97 +2968,97 @@ class NoNonVirtualDestructorsTest(CpplintTestBase):
def testNoDestructorWhenVirtualNeeded(self): def testNoDestructorWhenVirtualNeeded(self):
self.TestMultiLineLintRE( self.TestMultiLineLintRE(
'''class Foo { """class Foo {
virtual void foo(); virtual void foo();
};''', };""",
'The class Foo probably needs a virtual destructor') 'The class Foo probably needs a virtual destructor')
def testDestructorNonVirtualWhenVirtualNeeded(self): def testDestructorNonVirtualWhenVirtualNeeded(self):
self.TestMultiLineLintRE( self.TestMultiLineLintRE(
'''class Foo { """class Foo {
~Foo(); ~Foo();
virtual void foo(); virtual void foo();
};''', };""",
'The class Foo probably needs a virtual destructor') 'The class Foo probably needs a virtual destructor')
def testNoWarnWhenDerived(self): def testNoWarnWhenDerived(self):
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo : public Goo { """class Foo : public Goo {
virtual void foo(); virtual void foo();
};''', };""",
'') '')
def testNoDestructorWhenVirtualNeededClassDecorated(self): def testNoDestructorWhenVirtualNeededClassDecorated(self):
self.TestMultiLineLintRE( self.TestMultiLineLintRE(
'''class LOCKABLE API Foo { """class LOCKABLE API Foo {
virtual void foo(); virtual void foo();
};''', };""",
'The class Foo probably needs a virtual destructor') 'The class Foo probably needs a virtual destructor')
def testDestructorNonVirtualWhenVirtualNeededClassDecorated(self): def testDestructorNonVirtualWhenVirtualNeededClassDecorated(self):
self.TestMultiLineLintRE( self.TestMultiLineLintRE(
'''class LOCKABLE API Foo { """class LOCKABLE API Foo {
~Foo(); ~Foo();
virtual void foo(); virtual void foo();
};''', };""",
'The class Foo probably needs a virtual destructor') 'The class Foo probably needs a virtual destructor')
def testNoWarnWhenDerivedClassDecorated(self): def testNoWarnWhenDerivedClassDecorated(self):
self.TestMultiLineLint( self.TestMultiLineLint(
'''class LOCKABLE API Foo : public Goo { """class LOCKABLE API Foo : public Goo {
virtual void foo(); virtual void foo();
};''', };""",
'') '')
def testInternalBraces(self): def testInternalBraces(self):
self.TestMultiLineLintRE( self.TestMultiLineLintRE(
'''class Foo { """class Foo {
enum Goo { enum Goo {
GOO GOO
}; };
virtual void foo(); virtual void foo();
};''', };""",
'The class Foo probably needs a virtual destructor') 'The class Foo probably needs a virtual destructor')
def testInnerClassNeedsVirtualDestructor(self): def testInnerClassNeedsVirtualDestructor(self):
self.TestMultiLineLintRE( self.TestMultiLineLintRE(
'''class Foo { """class Foo {
class Goo { class Goo {
virtual void goo(); virtual void goo();
}; };
};''', };""",
'The class Goo probably needs a virtual destructor') 'The class Goo probably needs a virtual destructor')
def testOuterClassNeedsVirtualDestructor(self): def testOuterClassNeedsVirtualDestructor(self):
self.TestMultiLineLintRE( self.TestMultiLineLintRE(
'''class Foo { """class Foo {
class Goo { class Goo {
}; };
virtual void foo(); virtual void foo();
};''', };""",
'The class Foo probably needs a virtual destructor') 'The class Foo probably needs a virtual destructor')
def testQualifiedClassNeedsVirtualDestructor(self): def testQualifiedClassNeedsVirtualDestructor(self):
self.TestMultiLineLintRE( self.TestMultiLineLintRE(
'''class Qualified::Foo { """class Qualified::Foo {
virtual void foo(); virtual void foo();
};''', };""",
'The class Qualified::Foo probably needs a virtual destructor') 'The class Qualified::Foo probably needs a virtual destructor')
def testMultiLineDeclarationNoError(self): def testMultiLineDeclarationNoError(self):
self.TestMultiLineLintRE( self.TestMultiLineLintRE(
'''class Foo """class Foo
: public Goo { : public Goo {
virtual void foo(); virtual void foo();
};''', };""",
'') '')
def testMultiLineDeclarationWithError(self): def testMultiLineDeclarationWithError(self):
self.TestMultiLineLint( self.TestMultiLineLint(
'''class Foo """class Foo
{ {
virtual void foo(); virtual void foo();
};''', };""",
['{ should almost always be at the end of the previous line ' ['{ should almost always be at the end of the previous line '
'[whitespace/braces] [4]', '[whitespace/braces] [4]',
'The class Foo probably needs a virtual destructor due to having ' 'The class Foo probably needs a virtual destructor due to having '
@ -2925,10 +3070,22 @@ class NoNonVirtualDestructorsTest(CpplintTestBase):
'If you can, use sizeof(fisk) instead of 1 as the 2nd arg ' 'If you can, use sizeof(fisk) instead of 1 as the 2nd arg '
'to snprintf. [runtime/printf] [3]') 'to snprintf. [runtime/printf] [3]')
def testExplicitMakePair(self):
self.TestLint('make_pair', '')
self.TestLint('make_pair(42, 42)', '')
self.TestLint('make_pair<',
'Omit template arguments from make_pair OR use pair directly'
' OR if appropriate, construct a pair directly'
' [build/explicit_make_pair] [4]')
self.TestLint('make_pair <',
'Omit template arguments from make_pair OR use pair directly'
' OR if appropriate, construct a pair directly'
' [build/explicit_make_pair] [4]')
self.TestLint('my_make_pair<int, int>', '')
# pylint: disable-msg=C6409 # pylint: disable-msg=C6409
def setUp(): def setUp():
""" Runs before all tests are executed. """Runs before all tests are executed.
""" """
# Enable all filters, so we don't miss anything that is off by default. # Enable all filters, so we don't miss anything that is off by default.
cpplint._DEFAULT_FILTERS = [] cpplint._DEFAULT_FILTERS = []