Update cpplint.py to #387:
387 - Allow lint warnings on the next line to be suppressed with NOLINTNEXTLINE.
386 - Fixed false positive for strcat and strcpy.
385 - Fixed false positive for bitwise and on last line of a multi-line
384 - Remove unused deprecated testing/base/googletest.h header.
383 - Fixed false positive for bitwise and on last line of a multi-line
initializer list that ends with closing brace on the same line.
382 - Allow header guard warning to be silenced with
"// NOLINT(build/header_guard)" comment.
381 - Removed check for spaces between "//" and "TODO".
380 - Fix typo: Redudnant -> Redundant
379 - Added check for redundant "virtual" and "override" function-specifier.
378 - Updated missing "explicit" message to match style guide wording
377 - Add cpplint check to prevent .cc file inclusions from other packages.
These are generally bad and we should prevent regressions like this until
proper BUILD layering checks are in place.
376 - Silence missing braces warning for conditional statements inside lambdas.
375 - Warn about multi-arg constructors being explicit only at highest severity
level.
374 - Warn about non-single-argument constructors marked explicit.
373 - Exclude 'case' from function spacing check.
372 - Exclude third party headers such as Python.h from these checks:
371 - Fixed false positive for function pointer parameters followed by other
parameters.
370 - Allow RValue reference for default inline constructor.
369 - Fixed false positive for array of references and operator,().
368 - Silence named parameter warning for macros.
367 - Fixed false positive for duplicate header warning across preprocessor
boundaries.
366 - Allow rvalue references for constructors and assignment operator.
365 - Stop handling DISALLOW_EVIL_CONSTRUCTORS in cpplint.py,
since it's going away.
364 - Fixed false positive for member function in nested classes.
363 - Namespace indentation for cpplint. The C++ style guide enforced that items
within a namespace do not receive an extra level of indentation.
362 - Fixed false positive for multi-line member functions returning strings.
361 - Do not suggest replacement CHECK macros if custom error messages are used.
360 - Silence nonconst reference warning for multiline derived functions.
359 - Silence non-const reference warnings for derived functions.
358 - Added link to developer guide.
357 - Remove the partial ban on std::move and related features. More general
356 - Fixed false positive for << inside macros.
355 - Make _NestingState class public.
354 - Fixed false positive for multiline function templates.
353 - Fixed false positive for lambda capture.
352 - Silence RValue reference warnings that are enclosed in a
351 - Do not warn on CR-LF lines if all input lines are uniformly CR-LF.
350 - Tweaked the error messages for static mutex with default constructors.
349 - Fixed false positive for unnamed parameters in macros.
348 - Recognize &&... as RValue references.
347 - Use alternative error message for including <ostream>.
346 - Fixed false positive for function style template argument.
345 - Fixed false positive for braced constructor call inside brackets.
344 - Minor spelling and grammar fix.
343 - Fixed false positive for non-const reference check inside constructor
342 - Fixed cases where rvalue references are not identified correctly:
341 - Check that third party header files have some header guard, but do not
340 - Don't check header guards in third party files.
339 - Instead of treating thread annotations as macro function calls, skip over
them in parsing function signatures. This silences false positives for
lambdas with thread annotations.
338 - Fixed false positive for deprecated cast where return type of function
337 - Fixed false positive for alias-declarations of function pointers.
336 - Improved error message for taking address of something dereferenced
335 - Added support for C++14 digit separators.
334 - Silence long line warnings for JNI.
333 - Fixed false positive for constructor calls for rand{} being
332 - Elide C++11 raw strings as an empty quoted string instead of
331 - Split up long functions in cpplint.py and cpplint_unittest.py
330 - Fixed false positive for missing space check around "using operator;"
329 - Fixed false positive for indent check around multi-line raw strings.
R=erg@google.com
Review URL: https://codereview.appspot.com/125980043diff --git a/cpplint/cpplint.py b/cpplint/cpplint.py
index fca5d6a..e60e940 100755
--- a/cpplint/cpplint.py
+++ b/cpplint/cpplint.py
@@ -194,6 +194,7 @@
'readability/constructors',
'readability/fn_size',
'readability/function',
+ 'readability/inheritance',
'readability/multiline_comment',
'readability/multiline_string',
'readability/namespace',
@@ -210,6 +211,7 @@
'runtime/invalid_increment',
'runtime/member_string_references',
'runtime/memset',
+ 'runtime/indentation_namespace',
'runtime/operator',
'runtime/printf',
'runtime/printf_format',
@@ -383,6 +385,15 @@
])
+# These headers are excluded from [build/include] and [build/include_order]
+# checks:
+# - Anything not following google file name conventions (containing an
+# uppercase character, such as Python.h or nsStringAPI.h, for example).
+# - Lua headers.
+_THIRD_PARTY_HEADERS_PATTERN = re.compile(
+ r'^(?:[^/]*[A-Z][^/]*\.h|lua\.h|lauxlib\.h|lualib\.h)$')
+
+
# Assertion macros. These are defined in base/logging.h and
# testing/base/gunit.h. Note that the _M versions need to come first
# for substring matching to work.
@@ -465,9 +476,6 @@
_regexp_compile_cache = {}
-# Finds occurrences of NOLINT or NOLINT(...).
-_RE_SUPPRESSION = re.compile(r'\bNOLINT\b(\([^)]*\))?')
-
# {str, set(int)}: a map from error categories to sets of linenumbers
# on which those errors are expected and should be suppressed.
_error_suppressions = {}
@@ -497,24 +505,27 @@
linenum: int, the number of the current line.
error: function, an error handler.
"""
- # FIXME(adonovan): "NOLINT(" is misparsed as NOLINT(*).
- matched = _RE_SUPPRESSION.search(raw_line)
+ matched = Search(r'\bNOLINT(NEXTLINE)?\b(\([^)]+\))?', raw_line)
if matched:
- category = matched.group(1)
+ if matched.group(1):
+ suppressed_line = linenum + 1
+ else:
+ suppressed_line = linenum
+ category = matched.group(2)
if category in (None, '(*)'): # => "suppress all"
- _error_suppressions.setdefault(None, set()).add(linenum)
+ _error_suppressions.setdefault(None, set()).add(suppressed_line)
else:
if category.startswith('(') and category.endswith(')'):
category = category[1:-1]
if category in _ERROR_CATEGORIES:
- _error_suppressions.setdefault(category, set()).add(linenum)
+ _error_suppressions.setdefault(category, set()).add(suppressed_line)
else:
error(filename, linenum, 'readability/nolint', 5,
'Unknown NOLINT error category: %s' % category)
def ResetNolintSuppressions():
- "Resets the set of NOLINT suppressions to empty."
+ """Resets the set of NOLINT suppressions to empty."""
_error_suppressions.clear()
@@ -569,11 +580,12 @@
return _regexp_compile_cache[pattern].search(s)
-class _IncludeState(dict):
+class _IncludeState(object):
"""Tracks line numbers for includes, and the order in which includes appear.
- As a dict, an _IncludeState object serves as a mapping between include
- filename and line number on which that file was included.
+ include_list contains list of lists of (header, line number) pairs.
+ It's a lists of lists rather than just one flat list to make it
+ easier to update across preprocessor boundaries.
Call CheckNextIncludeOrder() once for each header in the file, passing
in the type constants defined above. Calls in an illegal order will
@@ -604,15 +616,42 @@
}
def __init__(self):
- dict.__init__(self)
- self.ResetSection()
+ self.include_list = [[]]
+ self.ResetSection('')
- def ResetSection(self):
+ def FindHeader(self, header):
+ """Check if a header has already been included.
+
+ Args:
+ header: header to check.
+ Returns:
+ Line number of previous occurrence, or -1 if the header has not
+ been seen before.
+ """
+ for section_list in self.include_list:
+ for f in section_list:
+ if f[0] == header:
+ return f[1]
+ return -1
+
+ def ResetSection(self, directive):
+ """Reset section checking for preprocessor directive.
+
+ Args:
+ directive: preprocessor directive (e.g. "if", "else").
+ """
# The name of the current section.
self._section = self._INITIAL_SECTION
# The path of last found header.
self._last_header = ''
+ # Update list of includes. Note that we never pop from the
+ # include list.
+ if directive in ('if', 'ifdef', 'ifndef'):
+ self.include_list.append([])
+ elif directive in ('else', 'elif'):
+ self.include_list[-1] = []
+
def SetLastHeader(self, header_path):
self._last_header = header_path
@@ -923,7 +962,7 @@
pass
-class FileInfo:
+class FileInfo(object):
"""Provides utility functions for filenames.
FileInfo provides easy access to the components of a file's path
@@ -1634,6 +1673,16 @@
error: The function to call with any errors found.
"""
+ # Don't check for header guards if there are error suppression
+ # comments somewhere in this file.
+ #
+ # Because this is silencing a warning for a nonexistent line, we
+ # only support the very specific NOLINT(build/header_guard) syntax,
+ # and not the general NOLINT or NOLINT(*) syntax.
+ for i in lines:
+ if Search(r'//\s*NOLINT\(build/header_guard\)', i):
+ return
+
cppvar = GetHeaderGuardCPPVariable(filename)
ifndef = None
@@ -1880,6 +1929,20 @@
'Changing pointer instead of value (or unused value of operator*).')
+def IsMacroDefinition(clean_lines, linenum):
+ if Search(r'^#define', clean_lines[linenum]):
+ return True
+
+ if linenum > 0 and Search(r'\\$', clean_lines[linenum - 1]):
+ return True
+
+ return False
+
+
+def IsForwardClassDeclaration(clean_lines, linenum):
+ return Match(r'^\s*(\btemplate\b)*.*class\s+\w+;\s*$', clean_lines[linenum])
+
+
class _BlockInfo(object):
"""Stores information about a generic block of code."""
@@ -1887,6 +1950,7 @@
self.seen_open_brace = seen_open_brace
self.open_parentheses = 0
self.inline_asm = _NO_ASM
+ self.check_namespace_indentation = False
def CheckBegin(self, filename, clean_lines, linenum, error):
"""Run checks that applies to text up to the opening brace.
@@ -1943,6 +2007,7 @@
self.name = name
self.starting_linenum = linenum
self.is_derived = False
+ self.check_namespace_indentation = True
if class_or_struct == 'struct':
self.access = 'public'
self.is_struct = True
@@ -1994,6 +2059,7 @@
_BlockInfo.__init__(self, False)
self.name = name or ''
self.starting_linenum = linenum
+ self.check_namespace_indentation = True
def CheckEnd(self, filename, clean_lines, linenum, error):
"""Check end of namespace comments."""
@@ -2531,17 +2597,73 @@
base_classname = classinfo.name.split('::')[-1]
# Look for single-argument constructors that aren't marked explicit.
- # Technically a valid construct, but against style.
- args = Match(r'\s+(?:inline\s+)?%s\s*\(([^,()]+)\)'
- % re.escape(base_classname),
- line)
- if (args and
- args.group(1) != 'void' and
- not Search(r'\bstd::initializer_list\b', args.group(1)) and
- not Match(r'(const\s+)?%s(\s+const)?\s*(?:<\w+>\s*)?&'
- % re.escape(base_classname), args.group(1).strip())):
- error(filename, linenum, 'runtime/explicit', 5,
- 'Single-argument constructors should be marked explicit.')
+ # Technically a valid construct, but against style. Also look for
+ # non-single-argument constructors which are also technically valid, but
+ # strongly suggest something is wrong.
+ explicit_constructor_match = Match(
+ r'\s+(?:inline\s+)?(explicit\s+)?(?:inline\s+)?%s\s*'
+ r'\(((?:[^()]|\([^()]*\))*)\)'
+ % re.escape(base_classname),
+ line)
+
+ if explicit_constructor_match:
+ is_marked_explicit = explicit_constructor_match.group(1)
+
+ if not explicit_constructor_match.group(2):
+ constructor_args = []
+ else:
+ constructor_args = explicit_constructor_match.group(2).split(',')
+
+ # collapse arguments so that commas in template parameter lists and function
+ # argument parameter lists don't split arguments in two
+ i = 0
+ while i < len(constructor_args):
+ constructor_arg = constructor_args[i]
+ while (constructor_arg.count('<') > constructor_arg.count('>') or
+ constructor_arg.count('(') > constructor_arg.count(')')):
+ constructor_arg += ',' + constructor_args[i + 1]
+ del constructor_args[i + 1]
+ constructor_args[i] = constructor_arg
+ i += 1
+
+ defaulted_args = [arg for arg in constructor_args if '=' in arg]
+ noarg_constructor = (not constructor_args or # empty arg list
+ # 'void' arg specifier
+ (len(constructor_args) == 1 and
+ constructor_args[0].strip() == 'void'))
+ onearg_constructor = ((len(constructor_args) == 1 and # exactly one arg
+ not noarg_constructor) or
+ # all but at most one arg defaulted
+ (len(constructor_args) >= 1 and
+ not noarg_constructor and
+ len(defaulted_args) >= len(constructor_args) - 1))
+ initializer_list_constructor = bool(
+ onearg_constructor and
+ Search(r'\bstd\s*::\s*initializer_list\b', constructor_args[0]))
+ copy_constructor = bool(
+ onearg_constructor and
+ Match(r'(const\s+)?%s(\s*<[^>]*>)?(\s+const)?\s*(?:<\w+>\s*)?&'
+ % re.escape(base_classname), constructor_args[0].strip()))
+
+ if (not is_marked_explicit and
+ onearg_constructor and
+ not initializer_list_constructor and
+ not copy_constructor):
+ if defaulted_args:
+ error(filename, linenum, 'runtime/explicit', 5,
+ 'Constructors callable with one argument '
+ 'should be marked explicit.')
+ else:
+ error(filename, linenum, 'runtime/explicit', 5,
+ 'Single-parameter constructors should be marked explicit.')
+ elif is_marked_explicit and not onearg_constructor:
+ if noarg_constructor:
+ error(filename, linenum, 'runtime/explicit', 5,
+ 'Zero-parameter constructors should not be marked explicit.')
+ else:
+ error(filename, linenum, 'runtime/explicit', 0,
+ 'Constructors that require multiple arguments '
+ 'should not be marked explicit.')
def CheckSpacingForFunctionCall(filename, clean_lines, linenum, error):
@@ -2634,6 +2756,20 @@
return not line or line.isspace()
+def CheckForNamespaceIndentation(filename, nesting_state, clean_lines, line,
+ error):
+ is_namespace_indent_item = (
+ len(nesting_state.stack) > 1 and
+ nesting_state.stack[-1].check_namespace_indentation and
+ isinstance(nesting_state.previous_stack_top, _NamespaceInfo) and
+ nesting_state.previous_stack_top == nesting_state.stack[-2])
+
+ if ShouldCheckNamespaceIndentation(nesting_state, is_namespace_indent_item,
+ clean_lines.elided, line):
+ CheckItemIndentationInNamespace(filename, clean_lines.elided,
+ line, error)
+
+
def CheckForFunctionLengths(filename, clean_lines, linenum,
function_state, error):
"""Reports for long function bodies.
@@ -2772,7 +2908,6 @@
line = clean_lines.elided[linenum] # get rid of comments and strings
matched = Match((r'\s*(DISALLOW_COPY_AND_ASSIGN|'
- r'DISALLOW_EVIL_CONSTRUCTORS|'
r'DISALLOW_IMPLICIT_CONSTRUCTORS)'), line)
if not matched:
return
@@ -2994,6 +3129,7 @@
# We allow no-spaces around << when used like this: 10<<20, but
# not otherwise (particularly, not when used as streams)
+ #
# We also allow operators following an opening parenthesis, since
# those tend to be macros that deal with operators.
match = Search(r'(operator|\S)(?:L|UL|ULL|l|ul|ull)?<<([^\s,=])', line)
@@ -3087,7 +3223,8 @@
# verify that lines contain missing whitespaces, second pass on raw
# lines to confirm that those missing whitespaces are not due to
# elided comments.
- if Search(r',[^,\s]', line) and Search(r',[^,\s]', raw[linenum]):
+ if (Search(r',[^,\s]', ReplaceAll(r'\boperator\s*,\s*\(', 'F(', line)) and
+ Search(r',[^,\s]', raw[linenum])):
error(filename, linenum, 'whitespace/comma', 3,
'Missing space after ,')
@@ -3392,7 +3529,7 @@
match_symbol.group(1))
if match_func:
# Check for constructors, which don't have return types.
- if Search(r'\bexplicit$', match_func.group(1)):
+ if Search(r'\b(?:explicit|inline)$', match_func.group(1)):
return True
implicit_constructor = Match(r'\s*(\w+)\((?:const\s+)?(\w+)', prefix)
if (implicit_constructor and
@@ -3417,8 +3554,27 @@
return False
+def IsDeletedOrDefault(clean_lines, linenum):
+ """Check if current constructor or operator is deleted or default.
+
+ Args:
+ clean_lines: A CleansedLines instance containing the file.
+ linenum: The number of the line to check.
+ Returns:
+ True if this is a deleted or default constructor.
+ """
+ open_paren = clean_lines.elided[linenum].find('(')
+ if open_paren < 0:
+ return False
+ (close_line, _, close_paren) = CloseExpression(
+ clean_lines, linenum, open_paren)
+ if close_paren < 0:
+ return False
+ return Match(r'\s*=\s*(?:delete|default)\b', close_line[close_paren:])
+
+
def IsRValueAllowed(clean_lines, linenum):
- """Check if RValue reference is allowed within some range of lines.
+ """Check if RValue reference is allowed on a particular line.
Args:
clean_lines: A CleansedLines instance containing the file.
@@ -3426,6 +3582,7 @@
Returns:
True if line is within the region where RValue references are allowed.
"""
+ # Allow region marked by PUSH/POP macros
for i in xrange(linenum, 0, -1):
line = clean_lines.elided[i]
if Match(r'GOOGLE_ALLOW_RVALUE_REFERENCES_(?:PUSH|POP)', line):
@@ -3435,6 +3592,26 @@
line = clean_lines.elided[j]
if Match(r'GOOGLE_ALLOW_RVALUE_REFERENCES_(?:PUSH|POP)', line):
return line.endswith('POP')
+
+ # Allow operator=
+ line = clean_lines.elided[linenum]
+ if Search(r'\boperator\s*=\s*\(', line):
+ return IsDeletedOrDefault(clean_lines, linenum)
+
+ # Allow constructors
+ match = Match(r'\s*([\w<>]+)\s*::\s*([\w<>]+)\s*\(', line)
+ if match and match.group(1) == match.group(2):
+ return IsDeletedOrDefault(clean_lines, linenum)
+ if Search(r'\b(?:explicit|inline)\s+[\w<>]+\s*\(', line):
+ return IsDeletedOrDefault(clean_lines, linenum)
+
+ if Match(r'\s*[\w<>]+\s*\(', line):
+ previous_line = 'ReturnType'
+ if linenum > 0:
+ previous_line = clean_lines.elided[linenum - 1]
+ if Match(r'^\s*$', previous_line) or Search(r'[{}:;]\s*$', previous_line):
+ return IsDeletedOrDefault(clean_lines, linenum)
+
return False
@@ -3643,9 +3820,13 @@
# methods) and a single \ after the semicolon (for macros)
endpos = endline.find(';')
if not Match(r';[\s}]*(\\?)$', endline[endpos:]):
- # Semicolon isn't the last character, there's something trailing
- error(filename, linenum, 'readability/braces', 4,
- 'If/else bodies with multiple statements require braces')
+ # Semicolon isn't the last character, there's something trailing.
+ # Output a warning if the semicolon is not contained inside
+ # a lambda expression.
+ if not Match(r'^[^{};]*\[[^\[\]]*\][^{}]*\{[^{}]*\}\s*\)*[;,]\s*$',
+ endline):
+ error(filename, linenum, 'readability/braces', 4,
+ 'If/else bodies with multiple statements require braces')
elif endlinenum < len(clean_lines.elided) - 1:
# Make sure the next line is dedented
next_line = clean_lines.elided[endlinenum + 1]
@@ -3876,6 +4057,13 @@
clean_lines, linenum, start_pos)
if end_pos < 0:
return
+
+ # If the check macro is followed by something other than a
+ # semicolon, assume users will log their own custom error messages
+ # and don't suggest any replacements.
+ if not Match(r'\s*;', last_line[end_pos:]):
+ return
+
if linenum == end_line:
expression = lines[linenum][start_pos + 1:end_pos - 1]
else:
@@ -4139,7 +4327,6 @@
CheckSectionSpacing(filename, clean_lines, classinfo, linenum, error)
-_RE_PATTERN_INCLUDE_NEW_STYLE = re.compile(r'#include +"[^/]+\.h"')
_RE_PATTERN_INCLUDE = re.compile(r'^\s*#\s*include\s*([<"])([^>"]*)[>"].*$')
# Matches the first component of a filename delimited by -s and _s. That is:
# _RE_FIRST_COMPONENT.match('foo').group(0) == 'foo'
@@ -4271,7 +4458,14 @@
line = clean_lines.lines[linenum]
# "include" should use the new style "foo/bar.h" instead of just "bar.h"
- if _RE_PATTERN_INCLUDE_NEW_STYLE.search(line):
+ # Only do this check if the included header follows google naming
+ # conventions. If not, assume that it's a 3rd party API that
+ # requires special include conventions.
+ #
+ # We also make an exception for Lua headers, which follow google
+ # naming convention but not the include convention.
+ match = Match(r'#include\s*"([^/]+\.h)"', line)
+ if match and not _THIRD_PARTY_HEADERS_PATTERN.match(match.group(1)):
error(filename, linenum, 'build/include', 4,
'Include the directory when naming .h files')
@@ -4282,12 +4476,13 @@
if match:
include = match.group(2)
is_system = (match.group(1) == '<')
- if include in include_state:
+ duplicate_line = include_state.FindHeader(include)
+ if duplicate_line >= 0:
error(filename, linenum, 'build/include', 4,
'"%s" already included at %s:%s' %
- (include, filename, include_state[include]))
- else:
- include_state[include] = linenum
+ (include, filename, duplicate_line))
+ elif not _THIRD_PARTY_HEADERS_PATTERN.match(include):
+ include_state.include_list[-1].append((include, linenum))
# We want to ensure that headers appear in the right order:
# 1) for foo.cc, foo.h (preferred location)
@@ -4441,8 +4636,9 @@
# Reset include state across preprocessor directives. This is meant
# to silence warnings for conditional includes.
- if Match(r'^\s*#\s*(?:ifdef|elif|else|endif)\b', line):
- include_state.ResetSection()
+ match = Match(r'^\s*#\s*(if|ifdef|ifndef|elif|else|endif)\b', line)
+ if match:
+ include_state.ResetSection(match.group(1))
# Make Windows paths like Unix.
fullname = os.path.abspath(filename).replace('\\', '/')
@@ -4456,7 +4652,7 @@
# TODO(unknown): check that 1-arg constructors are explicit.
# How to tell it's a constructor?
# (handled in CheckForNonStandardConstructs for now)
- # TODO(unknown): check that classes have DISALLOW_EVIL_CONSTRUCTORS
+ # TODO(unknown): check that classes declare or disable copy/assign
# (level 1 error)
pass
@@ -4556,12 +4752,11 @@
'Do not use variable-length arrays. Use an appropriately named '
"('k' followed by CamelCase) compile-time constant for the size.")
- # If DISALLOW_EVIL_CONSTRUCTORS, DISALLOW_COPY_AND_ASSIGN, or
- # DISALLOW_IMPLICIT_CONSTRUCTORS is present, then it should be the last thing
- # in the class declaration.
+ # If DISALLOW_COPY_AND_ASSIGN DISALLOW_IMPLICIT_CONSTRUCTORS is present,
+ # then it should be the last thing in the class declaration.
match = Match(
(r'\s*'
- r'(DISALLOW_(EVIL_CONSTRUCTORS|COPY_AND_ASSIGN|IMPLICIT_CONSTRUCTORS))'
+ r'(DISALLOW_(COPY_AND_ASSIGN|IMPLICIT_CONSTRUCTORS))'
r'\(.*\);$'),
line)
if match and linenum + 1 < clean_lines.NumLines():
@@ -4599,12 +4794,17 @@
"""
line = clean_lines.elided[linenum]
+ # Match two lines at a time to support multiline declarations
+ if linenum + 1 < clean_lines.NumLines() and not Search(r'[;({]', line):
+ line += clean_lines.elided[linenum + 1].strip()
+
# Check for people declaring static/global STL strings at the top level.
# This is dangerous because the C++ language does not guarantee that
# globals with constructors are initialized before the first access.
match = Match(
r'((?:|static +)(?:|const +))string +([a-zA-Z0-9_:]+)\b(.*)',
line)
+
# Remove false positives:
# - String pointers (as opposed to values).
# string *pointer
@@ -4624,7 +4824,7 @@
if (match and
not Search(r'\bstring\b(\s+const)?\s*\*\s*(const\s+)?\w', line) and
not Search(r'\boperator\W', line) and
- not Match(r'\s*(<.*>)?(::[a-zA-Z0-9_]+)?\s*\(([^"]|$)', match.group(3))):
+ not Match(r'\s*(<.*>)?(::[a-zA-Z0-9_]+)*\s*\(([^"]|$)', match.group(3))):
error(filename, linenum, 'runtime/string', 4,
'For a static/global string constant, use a C style string instead: '
'"%schar %s[]".' %
@@ -4655,10 +4855,10 @@
'to snprintf.' % (match.group(1), match.group(2)))
# Check if some verboten C functions are being used.
- if Search(r'\bsprintf\b', line):
+ if Search(r'\bsprintf\s*\(', line):
error(filename, linenum, 'runtime/printf', 5,
'Never use sprintf. Use snprintf instead.')
- match = Search(r'\b(strcpy|strcat)\b', line)
+ match = Search(r'\b(strcpy|strcat)\s*\(', line)
if match:
error(filename, linenum, 'runtime/printf', 4,
'Almost always, snprintf is better than %s' % match.group(1))
@@ -4674,13 +4874,16 @@
True if current line contains a function with "override"
virt-specifier.
"""
- # Look for leftmost opening parenthesis on current line
- opening_paren = clean_lines.elided[linenum].find('(')
- if opening_paren < 0: return False
-
- # Look for "override" after the matching closing parenthesis
- line, _, closing_paren = CloseExpression(clean_lines, linenum, opening_paren)
- return closing_paren >= 0 and Search(r'\boverride\b', line[closing_paren:])
+ # Scan back a few lines for start of current function
+ for i in xrange(linenum, max(-1, linenum - 10), -1):
+ match = Match(r'^([^()]*\w+)\(', clean_lines.elided[i])
+ if match:
+ # Look for "override" after the matching closing parenthesis
+ line, _, closing_paren = CloseExpression(
+ clean_lines, i, len(match.group(1)))
+ return (closing_paren >= 0 and
+ Search(r'\boverride\b', line[closing_paren:]))
+ return False
def IsInitializerList(clean_lines, linenum):
@@ -4806,6 +5009,20 @@
# Not at toplevel, not within a class, and not within a namespace
return
+ # Avoid initializer lists. We only need to scan back from the
+ # current line for something that starts with ':'.
+ #
+ # We don't need to check the current line, since the '&' would
+ # appear inside the second set of parentheses on the current line as
+ # opposed to the first set.
+ if linenum > 0:
+ for i in xrange(linenum - 1, max(0, linenum - 10), -1):
+ previous_line = clean_lines.elided[i]
+ if not Search(r'[),]\s*$', previous_line):
+ break
+ if Match(r'^\s*:\s+\S', previous_line):
+ return
+
# Avoid preprocessors
if Search(r'\\\s*$', line):
return
@@ -4881,6 +5098,11 @@
# value < double(42) // bracket + space = true positive
matched_new_or_template = match.group(1)
+ # Avoid arrays by looking for brackets that come after the closing
+ # parenthesis.
+ if Match(r'\([^()]+\)\s*\[', match.group(3)):
+ return
+
# Other things to ignore:
# - Function pointers
# - Casts to pointer types
@@ -4900,28 +5122,35 @@
matched_type)
if not expecting_function:
- CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum],
- 'static_cast',
+ CheckCStyleCast(filename, clean_lines, linenum, 'static_cast',
r'\((int|float|double|bool|char|u?int(16|32|64))\)', error)
# This doesn't catch all cases. Consider (const char * const)"hello".
#
# (char *) "foo" should always be a const_cast (reinterpret_cast won't
# compile).
- if CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum],
- 'const_cast', r'\((char\s?\*+\s?)\)\s*"', error):
+ if CheckCStyleCast(filename, clean_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)
+ CheckCStyleCast(filename, clean_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.
+ #
+ # Some non-identifier character is required before the '&' for the
+ # expression to be recognized as a cast. These are casts:
+ # expression = &static_cast<int*>(temporary());
+ # function(&(int*)(temporary()));
+ #
+ # This is not a cast:
+ # reference_type&(int* function_param);
match = Search(
- r'(?:&\(([^)]+)\)[\w(])|'
- r'(?:&(static|dynamic|down|reinterpret)_cast\b)', line)
+ r'(?:[^\w]&\(([^)]+)\)[\w(])|'
+ r'(?:[^\w]&(static|dynamic|down|reinterpret)_cast\b)', line)
if match and match.group(1) != '*':
# Try a better error message when the & is bound to something
# dereferenced by the casted pointer, as opposed to the casted
@@ -4951,15 +5180,13 @@
'Take the address before doing the cast, rather than after'))
-def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern,
- error):
+def CheckCStyleCast(filename, clean_lines, linenum, cast_type, pattern, error):
"""Checks for a C-style cast by looking for the pattern.
Args:
filename: The name of the current file.
+ clean_lines: A CleansedLines instance containing the file.
linenum: The number of the line to check.
- 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, static_cast, or const_cast, depending.
pattern: The regular expression used to find C-style casts.
@@ -4969,19 +5196,26 @@
True if an error was emitted.
False otherwise.
"""
+ line = clean_lines.elided[linenum]
match = Search(pattern, line)
if not match:
return False
- # Exclude lines with keywords that tend to look like casts, and also
- # macros which are generally troublesome.
- if Match(r'.*\b(?:sizeof|alignof|alignas|[A-Z_]+)\s*$',
- line[0:match.start(1) - 1]):
+ # Exclude lines with keywords that tend to look like casts
+ context = line[0:match.start(1) - 1]
+ if Match(r'.*\b(?:sizeof|alignof|alignas|[_A-Z][_A-Z0-9]*)\s*$', context):
+ return False
+
+ # Try expanding current context to see if we one level of
+ # parentheses inside a macro.
+ if linenum > 0:
+ for i in xrange(linenum - 1, max(0, linenum - 5), -1):
+ context = clean_lines.elided[i] + context
+ if Match(r'.*\b[_A-Z][_A-Z0-9]*\s*\((?:\([^()]*\)|[^()])*$', context):
return False
# operator++(int) and operator--(int)
- if (line[0:match.start(1) - 1].endswith(' operator++') or
- line[0:match.start(1) - 1].endswith(' operator--')):
+ if context.endswith(' operator++') or context.endswith(' operator--'):
return False
# A single unnamed argument for a function tends to look like old
@@ -5005,10 +5239,11 @@
# (FunctionPointer)(int);
# (FunctionPointer)(int) = value;
# Function((function_pointer_arg)(int))
+ # Function((function_pointer_arg)(int), int param)
# <TemplateArgument(int)>;
# <(FunctionPointerTemplateArgument)(int)>;
remainder = line[match.end(0):]
- if Match(r'^\s*(?:;|const\b|throw\b|final\b|override\b|=|>|\{|\))',
+ if Match(r'^\s*(?:;|const\b|throw\b|final\b|override\b|[=>{),])',
remainder):
# Looks like an unnamed parameter.
@@ -5031,6 +5266,7 @@
# Don't warn if the parameter is named with block comments, e.g.:
# Function(int /*unused_param*/);
+ raw_line = clean_lines.raw_lines[linenum]
if '/*' in raw_line:
return False
@@ -5182,16 +5418,16 @@
return files_belong_to_same_module, common_path
-def UpdateIncludeState(filename, include_state, io=codecs):
- """Fill up the include_state with new includes found from the file.
+def UpdateIncludeState(filename, include_dict, io=codecs):
+ """Fill up the include_dict with new includes found from the file.
Args:
filename: the name of the header to read.
- include_state: an _IncludeState instance in which the headers are inserted.
+ include_dict: a dictionary in which the headers are inserted.
io: The io factory to use to read the file. Provided for testability.
Returns:
- True if a header was succesfully added. False otherwise.
+ True if a header was successfully added. False otherwise.
"""
headerfile = None
try:
@@ -5205,9 +5441,7 @@
match = _RE_PATTERN_INCLUDE.search(clean_line)
if match:
include = match.group(2)
- # The value formatting is cute, but not really used right now.
- # What matters here is that the key is in include_state.
- include_state.setdefault(include, '%s:%d' % (filename, linenum))
+ include_dict.setdefault(include, linenum)
return True
@@ -5260,10 +5494,11 @@
# The policy is that if you #include something in foo.h you don't need to
# include it again in foo.cc. Here, we will look at possible includes.
- # Let's copy the include_state so it is only messed up within this function.
- include_state = include_state.copy()
+ # Let's flatten the include_state include_list and copy it into a dictionary.
+ include_dict = dict([item for sublist in include_state.include_list
+ for item in sublist])
- # Did we find the header for this file (if any) and succesfully load it?
+ # Did we find the header for this file (if any) and successfully load it?
header_found = False
# Use the absolute path so that matching works properly.
@@ -5278,13 +5513,13 @@
# instead of 'foo_flymake.h'
abs_filename = re.sub(r'_flymake\.cc$', '.cc', abs_filename)
- # include_state is modified during iteration, so we iterate over a copy of
+ # include_dict is modified during iteration, so we iterate over a copy of
# the keys.
- header_keys = include_state.keys()
+ header_keys = include_dict.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):
+ if same_module and UpdateIncludeState(fullpath, include_dict, io):
header_found = True
# If we can't find the header file for a .cc, assume it's because we don't
@@ -5298,7 +5533,7 @@
# All the lines have been processed, report the errors found.
for required_header_unstripped in required:
template = required[required_header_unstripped][1]
- if required_header_unstripped.strip('<>"') not in include_state:
+ if required_header_unstripped.strip('<>"') not in include_dict:
error(filename, required[required_header_unstripped][0],
'build/include_what_you_use', 4,
'Add #include ' + required_header_unstripped + ' for ' + template)
@@ -5326,6 +5561,8 @@
4, # 4 = high confidence
'For C++11-compatibility, omit template arguments from make_pair'
' OR use pair directly OR if appropriate, construct a pair directly')
+
+
def CheckDefaultLambdaCaptures(filename, clean_lines, linenum, error):
"""Check that default lambda captures are not used.
@@ -5351,6 +5588,139 @@
'Default lambda captures are an unapproved C++ feature.')
+def CheckRedundantVirtual(filename, clean_lines, linenum, error):
+ """Check if line contains a redundant "virtual" function-specifier.
+
+ 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.
+ """
+ # Look for "virtual" on current line.
+ line = clean_lines.elided[linenum]
+ virtual = Match(r'^(.*\bvirtual\b)', line)
+ if not virtual: return
+
+ # Look for the next opening parenthesis. This is the start of the
+ # parameter list (possibly on the next line shortly after virtual).
+ # TODO(unknown): doesn't work if there are virtual functions with
+ # decltype() or other things that use parentheses, but csearch suggests
+ # that this is rare.
+ end_col = -1
+ end_line = -1
+ start_col = len(virtual.group(1))
+ for start_line in xrange(linenum, min(linenum + 3, clean_lines.NumLines())):
+ line = clean_lines.elided[start_line][start_col:]
+ parameter_list = Match(r'^([^(]*)\(', line)
+ if parameter_list:
+ # Match parentheses to find the end of the parameter list
+ (_, end_line, end_col) = CloseExpression(
+ clean_lines, start_line, start_col + len(parameter_list.group(1)))
+ break
+ start_col = 0
+
+ if end_col < 0:
+ return # Couldn't find end of parameter list, give up
+
+ # Look for "override" or "final" after the parameter list
+ # (possibly on the next few lines).
+ for i in xrange(end_line, min(end_line + 3, clean_lines.NumLines())):
+ line = clean_lines.elided[i][end_col:]
+ match = Search(r'\b(override|final)\b', line)
+ if match:
+ error(filename, linenum, 'readability/inheritance', 4,
+ ('"virtual" is redundant since function is '
+ 'already declared as "%s"' % match.group(1)))
+
+ # Set end_col to check whole lines after we are done with the
+ # first line.
+ end_col = 0
+ if Search(r'[^\w]\s*$', line):
+ break
+
+
+def CheckRedundantOverrideOrFinal(filename, clean_lines, linenum, error):
+ """Check if line contains a redundant "override" or "final" virt-specifier.
+
+ 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.
+ """
+ # Check that at most one of "override" or "final" is present, not both
+ line = clean_lines.elided[linenum]
+ if Search(r'\boverride\b', line) and Search(r'\bfinal\b', line):
+ error(filename, linenum, 'readability/inheritance', 4,
+ ('"override" is redundant since function is '
+ 'already declared as "final"'))
+
+
+
+
+# Returns true if we are at a new block, and it is directly
+# inside of a namespace.
+def IsBlockInNameSpace(nesting_state, is_forward_declaration):
+ """Checks that the new block is directly in a namespace.
+
+ Args:
+ nesting_state: The _NestingState object that contains info about our state.
+ is_forward_declaration: If the class is a forward declared class.
+ Returns:
+ Whether or not the new block is directly in a namespace.
+ """
+ if is_forward_declaration:
+ if len(nesting_state.stack) >= 1 and (
+ isinstance(nesting_state.stack[-1], _NamespaceInfo)):
+ return True
+ else:
+ return False
+
+ return (len(nesting_state.stack) > 1 and
+ nesting_state.stack[-1].check_namespace_indentation and
+ isinstance(nesting_state.stack[-2], _NamespaceInfo))
+
+
+def ShouldCheckNamespaceIndentation(nesting_state, is_namespace_indent_item,
+ raw_lines_no_comments, linenum):
+ """This method determines if we should apply our namespace indentation check.
+
+ Args:
+ nesting_state: The current nesting state.
+ is_namespace_indent_item: If we just put a new class on the stack, True.
+ If the top of the stack is not a class, or we did not recently
+ add the class, False.
+ raw_lines_no_comments: The lines without the comments.
+ linenum: The current line number we are processing.
+
+ Returns:
+ True if we should apply our namespace indentation check. Currently, it
+ only works for classes and namespaces inside of a namespace.
+ """
+
+ is_forward_declaration = IsForwardClassDeclaration(raw_lines_no_comments,
+ linenum)
+
+ if not (is_namespace_indent_item or is_forward_declaration):
+ return False
+
+ # If we are in a macro, we do not want to check the namespace indentation.
+ if IsMacroDefinition(raw_lines_no_comments, linenum):
+ return False
+
+ return IsBlockInNameSpace(nesting_state, is_forward_declaration)
+
+
+# Call this method if the line is directly inside of a namespace.
+# If the line above is blank (excluding comments) or the start of
+# an inner namespace, it cannot be indented.
+def CheckItemIndentationInNamespace(filename, raw_lines_no_comments, linenum,
+ error):
+ line = raw_lines_no_comments[linenum]
+ if Match(r'^\s+', line):
+ error(filename, linenum, 'runtime/indentation_namespace', 4,
+ 'Do not indent within a namespace')
def ProcessLine(filename, file_extension, clean_lines, line,
@@ -5377,6 +5747,8 @@
raw_lines = clean_lines.raw_lines
ParseNolintSuppressions(filename, raw_lines[line], line, error)
nesting_state.Update(filename, clean_lines, line, error)
+ CheckForNamespaceIndentation(filename, nesting_state, clean_lines, line,
+ error)
if nesting_state.InAsmBlock(): return
CheckForFunctionLengths(filename, clean_lines, line, function_state, error)
CheckForMultilineCommentsAndStrings(filename, clean_lines, line, error)
@@ -5391,6 +5763,8 @@
CheckInvalidIncrement(filename, clean_lines, line, error)
CheckMakePairUsesDeduction(filename, clean_lines, line, error)
CheckDefaultLambdaCaptures(filename, clean_lines, line, error)
+ CheckRedundantVirtual(filename, clean_lines, line, error)
+ CheckRedundantOverrideOrFinal(filename, clean_lines, line, error)
for check_fn in extra_check_functions:
check_fn(filename, clean_lines, line, error)
diff --git a/cpplint/cpplint_unittest.py b/cpplint/cpplint_unittest.py
index 8a66def..c380696 100755
--- a/cpplint/cpplint_unittest.py
+++ b/cpplint/cpplint_unittest.py
@@ -37,14 +37,16 @@
import os
import random
import re
+import sys
import unittest
+
import cpplint
# This class works as an error collector and replaces cpplint.Error
# function for the unit tests. We also verify each category we see
# is in cpplint._ERROR_CATEGORIES, to help keep that list up to date.
-class ErrorCollector:
+class ErrorCollector(object):
# These are a global list, covering all categories seen ever.
_ERROR_CATEGORIES = cpplint._ERROR_CATEGORIES
_SEEN_ERROR_CATEGORIES = {}
@@ -74,7 +76,7 @@
return self._errors
def VerifyAllCategoriesAreSeen(self):
- """Fails if there's a category in _ERROR_CATEGORIES - _SEEN_ERROR_CATEGORIES.
+ """Fails if there's a category in _ERROR_CATEGORIES~_SEEN_ERROR_CATEGORIES.
This should only be called after all tests are run, so
_SEEN_ERROR_CATEGORIES has had a chance to fully populate. Since
@@ -84,7 +86,6 @@
"""
for category in self._ERROR_CATEGORIES:
if category not in self._SEEN_ERROR_CATEGORIES:
- import sys
sys.exit('FATAL ERROR: There are no tests for category "%s"' % category)
def RemoveIfPresent(self, substr):
@@ -96,7 +97,7 @@
# This class is a lame mock of codecs. We do not verify filename, mode, or
# encoding, but for the current use case it is not needed.
-class MockIo:
+class MockIo(object):
def __init__(self, mock_file):
self.mock_file = mock_file
@@ -109,6 +110,13 @@
class CpplintTestBase(unittest.TestCase):
"""Provides some useful helper functions for cpplint tests."""
+ def setUp(self):
+ # Allow subclasses to cheat os.path.abspath called in FileInfo class.
+ self.os_path_abspath_orig = os.path.abspath
+
+ def tearDown(self):
+ os.path.abspath = self.os_path_abspath_orig
+
# Perform lint on single line of input and return the error message.
def PerformSingleLineLint(self, code):
error_collector = ErrorCollector(self.assert_)
@@ -376,6 +384,15 @@
'long a = 65; // NOLINT(readability/casting)',
'Use int16/int64/etc, rather than the C type long'
' [runtime/int] [4]')
+ # NOLINTNEXTLINE silences warning for the next line instead of current line
+ error_collector = ErrorCollector(self.assert_)
+ cpplint.ProcessFileData('test.cc', 'cc',
+ ['// Copyright 2014 Your Company.',
+ '// NOLINTNEXTLINE(whitespace/line_length)',
+ '// ./command' + (' -verbose' * 80),
+ ''],
+ error_collector)
+ self.assertEquals('', error_collector.Results())
# Test Variable Declarations.
def testVariableDeclarations(self):
@@ -499,8 +516,9 @@
self.TestLint('void (*func)(void*);', '')
self.TestLint('void Func((*func)(void*)) {}', '')
self.TestLint('template <void Func(void*)> void func();', '')
- self.TestLint('virtual void f(int /*unused*/) override {', '')
- self.TestLint('virtual void f(int /*unused*/) final {', '')
+ self.TestLint('virtual void f(int /*unused*/) {', '')
+ self.TestLint('void f(int /*unused*/) override {', '')
+ self.TestLint('void f(int /*unused*/) final {', '')
# Test deprecated casts such as int(d)
def testDeprecatedCast(self):
@@ -523,81 +541,48 @@
' [readability/casting] [4]')
# Checks for false positives...
- self.TestLint(
- 'int a = int(); // Constructor, o.k.',
- '')
- self.TestLint(
- 'X::X() : a(int()) {} // default Constructor, o.k.',
- '')
- self.TestLint(
- 'operator bool(); // Conversion operator, o.k.',
- '')
- self.TestLint(
- 'new int64(123); // "new" operator on basic type, o.k.',
- '')
- self.TestLint(
- 'new int64(123); // "new" operator on basic type, weird spacing',
- '')
- self.TestLint(
- 'using a = bool(int arg); // C++11 alias-declaration',
- '')
- self.TestLint(
- 'std::function<int(bool)> // C++11 function wrapper',
- '')
- self.TestLint(
- 'std::function<const int(bool)>',
- '')
- self.TestLint(
- 'std::function< int(bool) >',
- '')
- self.TestLint(
- 'mfunction<int(bool)>',
- '')
+ self.TestLint('int a = int();', '') # constructor
+ self.TestLint('X::X() : a(int()) {}', '') # default constructor
+ self.TestLint('operator bool();', '') # Conversion operator
+ self.TestLint('new int64(123);', '') # "new" operator on basic type
+ self.TestLint('new int64(123);', '') # "new" operator on basic type
+ self.TestLint('using a = bool(int arg);', '') # C++11 alias-declaration
+ self.TestLint('x = bit_cast<double(*)[3]>(y);', '') # array of array
+ self.TestLint('void F(const char(&src)[N]);', '') # array of references
+
+ # Placement new
self.TestLint(
'new(field_ptr) int(field->default_value_enum()->number());',
'')
+ # C++11 function wrappers
+ self.TestLint('std::function<int(bool)>', '')
+ self.TestLint('std::function<const int(bool)>', '')
+ self.TestLint('std::function< int(bool) >', '')
+ self.TestLint('mfunction<int(bool)>', '')
+
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData(
'test.cc', 'cc',
- ['// Copyright 2008 Your Company. All Rights Reserved.',
+ ['// Copyright 2014 Your Company. All Rights Reserved.',
'typedef std::function<',
' bool(int)> F;',
''],
error_collector)
self.assertEquals('', error_collector.Results())
- # Return types for function pointers tend to look like casts.
- self.TestLint(
- 'typedef bool(FunctionPointer)();',
- '')
- self.TestLint(
- 'typedef bool(FunctionPointer)(int param);',
- '')
- self.TestLint(
- 'typedef bool(MyClass::*MemberFunctionPointer)();',
- '')
- self.TestLint(
- 'typedef bool(MyClass::* MemberFunctionPointer)();',
- '')
- self.TestLint(
- 'typedef bool(MyClass::*MemberFunctionPointer)() const;',
- '')
- self.TestLint(
- 'void Function(bool(FunctionPointerArg)());',
- '')
- self.TestLint(
- 'void Function(bool(FunctionPointerArg)()) {}',
- '')
- self.TestLint(
- 'typedef set<int64, bool(*)(int64, int64)> SortedIdSet',
- '')
+ # Return types for function pointers
+ self.TestLint('typedef bool(FunctionPointer)();', '')
+ self.TestLint('typedef bool(FunctionPointer)(int param);', '')
+ self.TestLint('typedef bool(MyClass::*MemberFunctionPointer)();', '')
+ self.TestLint('typedef bool(MyClass::* MemberFunctionPointer)();', '')
+ self.TestLint('typedef bool(MyClass::*MemberFunctionPointer)() const;', '')
+ self.TestLint('void Function(bool(FunctionPointerArg)());', '')
+ self.TestLint('void Function(bool(FunctionPointerArg)()) {}', '')
+ self.TestLint('typedef set<int64, bool(*)(int64, int64)> SortedIdSet', '')
self.TestLint(
'bool TraverseNode(T *Node, bool(VisitorBase:: *traverse) (T *t)) {}',
'')
- self.TestLint(
- 'x = bit_cast<double(*)[3]>(y);',
- '')
# The second parameter to a gMock method definition is a function signature
# that often looks like a bad cast but should not picked up by lint.
@@ -991,6 +976,8 @@
'Could not find end of multi-line comment'
' [readability/multiline_comment] [5]')
self.TestMultiLineLint(r""" // /* comment, but not multi-line""", '')
+ self.TestMultiLineLint(r"""/**********
+ */""", '')
def testMultilineStrings(self):
multiline_string_error_message = (
@@ -1012,192 +999,283 @@
# Test non-explicit single-argument constructors
def testExplicitSingleArgumentConstructors(self):
- # missing explicit is bad
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(int f);
- };""",
- 'Single-argument constructors should be marked explicit.'
- ' [runtime/explicit] [5]')
- # missing explicit is bad, even with whitespace
- self.TestMultiLineLint(
- """
- class Foo {
- Foo (int f);
- };""",
- ['Extra space before ( in function call [whitespace/parens] [4]',
- 'Single-argument constructors should be marked explicit.'
- ' [runtime/explicit] [5]'])
- # missing explicit, with distracting comment, is still bad
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(int f); // simpler than Foo(blargh, blarg)
- };""",
- 'Single-argument constructors should be marked explicit.'
- ' [runtime/explicit] [5]')
- # missing explicit, with qualified classname
- self.TestMultiLineLint(
- """
- class Qualifier::AnotherOne::Foo {
- 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.'
- ' [runtime/explicit] [5]')
- # structs are caught as well.
- self.TestMultiLineLint(
- """
- struct Foo {
- Foo(int f);
- };""",
- 'Single-argument constructors should be marked explicit.'
- ' [runtime/explicit] [5]')
- # Templatized classes are caught as well.
- self.TestMultiLineLint(
- """
- template<typename T> class Foo {
- 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.'
- ' [runtime/explicit] [5]')
- # proper style is okay
- self.TestMultiLineLint(
- """
- class Foo {
- explicit Foo(int f);
- };""",
- '')
- # two argument constructor is okay
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(int f, int b);
- };""",
- '')
- # two argument constructor, across two lines, is okay
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(int f,
- int b);
- };""",
- '')
- # non-constructor (but similar name), is okay
- self.TestMultiLineLint(
- """
- class Foo {
- aFoo(int f);
- };""",
- '')
- # constructor with void argument is okay
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(void);
- };""",
- '')
- # single argument method is okay
- self.TestMultiLineLint(
- """
- class Foo {
- Bar(int b);
- };""",
- '')
- # comments should be ignored
- self.TestMultiLineLint(
- """
- class Foo {
- // Foo(int f);
- };""",
- '')
- # single argument function following class definition is okay
- # (okay, it's not actually valid, but we don't want a false positive)
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(int f, int b);
- };
- Foo(int f);""",
- '')
- # single argument function is okay
- self.TestMultiLineLint(
- """static Foo(int f);""",
- '')
- # single argument copy constructor is okay.
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(const Foo&);
- };""",
- '')
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(Foo const&);
- };""",
- '')
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(Foo&);
- };""",
- '')
- # templatized copy constructor is okay.
- self.TestMultiLineLint(
- """
- template<typename T> class Foo {
- Foo(const Foo<T>&);
- };""",
- '')
- # Special case for std::initializer_list
- self.TestMultiLineLint(
- """
- class Foo {
- Foo(std::initializer_list<T> &arg) {}
- };""",
- '')
- # Anything goes inside an assembly block
- error_collector = ErrorCollector(self.assert_)
- cpplint.ProcessFileData('foo.cc', 'cc',
- ['void Func() {',
- ' __asm__ (',
- ' "hlt"',
- ' );',
- ' asm {',
- ' movdqa [edx + 32], xmm2',
- ' }',
- '}'],
- error_collector)
- self.assertEquals(
- 0,
- error_collector.ResultList().count(
- 'Extra space before ( in function call [whitespace/parens] [4]'))
- self.assertEquals(
- 0,
- error_collector.ResultList().count(
- 'Closing ) should be moved to the previous line '
- '[whitespace/parens] [2]'))
- self.assertEquals(
- 0,
- error_collector.ResultList().count(
- 'Extra space before [ [whitespace/braces] [5]'))
+ old_verbose_level = cpplint._cpplint_state.verbose_level
+ cpplint._cpplint_state.verbose_level = 0
+
+ try:
+ # missing explicit is bad
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(int f);
+ };""",
+ 'Single-parameter constructors should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # missing explicit is bad, even with whitespace
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo (int f);
+ };""",
+ ['Extra space before ( in function call [whitespace/parens] [4]',
+ 'Single-parameter constructors should be marked explicit.'
+ ' [runtime/explicit] [5]'])
+ # missing explicit, with distracting comment, is still bad
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(int f); // simpler than Foo(blargh, blarg)
+ };""",
+ 'Single-parameter constructors should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # missing explicit, with qualified classname
+ self.TestMultiLineLint(
+ """
+ class Qualifier::AnotherOne::Foo {
+ Foo(int f);
+ };""",
+ 'Single-parameter 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-parameter constructors should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # structs are caught as well.
+ self.TestMultiLineLint(
+ """
+ struct Foo {
+ Foo(int f);
+ };""",
+ 'Single-parameter constructors should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # Templatized classes are caught as well.
+ self.TestMultiLineLint(
+ """
+ template<typename T> class Foo {
+ Foo(int f);
+ };""",
+ 'Single-parameter 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-parameter constructors should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # constructors with a default argument should still be marked explicit
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(int f = 0);
+ };""",
+ 'Constructors callable with one argument should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # multi-argument constructors with all but one default argument should be
+ # marked explicit
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(int f, int g = 0);
+ };""",
+ 'Constructors callable with one argument should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # multi-argument constructors with all default arguments should be marked
+ # explicit
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(int f = 0, int g = 0);
+ };""",
+ 'Constructors callable with one argument should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # explicit no-argument constructors are bad
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ explicit Foo();
+ };""",
+ 'Zero-parameter constructors should not be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # void constructors are considered no-argument
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ explicit Foo(void);
+ };""",
+ 'Zero-parameter constructors should not be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # Warn explicit multi-argument constructors at lowest severity
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ explicit Foo(int f, int g);
+ };""",
+ 'Constructors that require multiple arguments '
+ 'should not be marked explicit. [runtime/explicit] [0]')
+ # but explicit multi-argument constructors with only one non-default
+ # argument are OK
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ explicit Foo(int f, int g = 0);
+ };""",
+ '')
+ # single-argument constructors that take a function that takes multiple
+ # arguments should be explicit
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(void (*f)(int f, int g));
+ };""",
+ 'Single-parameter constructors should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # single-argument constructors that take a single template argument with
+ # multiple parameters should be explicit
+ self.TestMultiLineLint(
+ """
+ template <typename T, typename S>
+ class Foo {
+ Foo(Bar<T, S> b);
+ };""",
+ 'Single-parameter constructors should be marked explicit.'
+ ' [runtime/explicit] [5]')
+ # but copy constructors that take multiple template parameters are OK
+ self.TestMultiLineLint(
+ """
+ template <typename T, S>
+ class Foo {
+ Foo(Foo<T, S>& f);
+ };""",
+ '')
+ # proper style is okay
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ explicit Foo(int f);
+ };""",
+ '')
+ # two argument constructor is okay
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(int f, int b);
+ };""",
+ '')
+ # two argument constructor, across two lines, is okay
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(int f,
+ int b);
+ };""",
+ '')
+ # non-constructor (but similar name), is okay
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ aFoo(int f);
+ };""",
+ '')
+ # constructor with void argument is okay
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(void);
+ };""",
+ '')
+ # single argument method is okay
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Bar(int b);
+ };""",
+ '')
+ # comments should be ignored
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ // Foo(int f);
+ };""",
+ '')
+ # single argument function following class definition is okay
+ # (okay, it's not actually valid, but we don't want a false positive)
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(int f, int b);
+ };
+ Foo(int f);""",
+ '')
+ # single argument function is okay
+ self.TestMultiLineLint(
+ """static Foo(int f);""",
+ '')
+ # single argument copy constructor is okay.
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(const Foo&);
+ };""",
+ '')
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(Foo const&);
+ };""",
+ '')
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(Foo&);
+ };""",
+ '')
+ # templatized copy constructor is okay.
+ self.TestMultiLineLint(
+ """
+ template<typename T> class Foo {
+ Foo(const Foo<T>&);
+ };""",
+ '')
+ # Special case for std::initializer_list
+ self.TestMultiLineLint(
+ """
+ class Foo {
+ Foo(std::initializer_list<T> &arg) {}
+ };""",
+ '')
+ # Anything goes inside an assembly block
+ error_collector = ErrorCollector(self.assert_)
+ cpplint.ProcessFileData('foo.cc', 'cc',
+ ['void Func() {',
+ ' __asm__ (',
+ ' "hlt"',
+ ' );',
+ ' asm {',
+ ' movdqa [edx + 32], xmm2',
+ ' }',
+ '}'],
+ error_collector)
+ self.assertEquals(
+ 0,
+ error_collector.ResultList().count(
+ 'Extra space before ( in function call [whitespace/parens] [4]'))
+ self.assertEquals(
+ 0,
+ error_collector.ResultList().count(
+ 'Closing ) should be moved to the previous line '
+ '[whitespace/parens] [2]'))
+ self.assertEquals(
+ 0,
+ error_collector.ResultList().count(
+ 'Extra space before [ [whitespace/braces] [5]'))
+ finally:
+ cpplint._cpplint_state.verbose_level = old_verbose_level
def testSlashStarCommentOnSingleLine(self):
self.TestMultiLineLint(
@@ -1269,6 +1347,44 @@
' memset(buf, 0xcd, 0)',
'')
+ def testRedundantVirtual(self):
+ self.TestLint('virtual void F()', '')
+ self.TestLint('virtual void F();', '')
+ self.TestLint('virtual void F() {}', '')
+
+ message_template = ('"%s" is redundant since function is already '
+ 'declared as "%s" [readability/inheritance] [4]')
+ for virt_specifier in ['override', 'final']:
+ error_message = message_template % ('virtual', virt_specifier)
+ self.TestLint('virtual int F() %s' % virt_specifier, error_message)
+ self.TestLint('virtual int F() %s;' % virt_specifier, error_message)
+ self.TestLint('virtual int F() %s {' % virt_specifier, error_message)
+
+ error_collector = ErrorCollector(self.assert_)
+ cpplint.ProcessFileData(
+ 'foo.cc', 'cc',
+ ['// Copyright 2014 Your Company.',
+ 'virtual void F(int a,',
+ ' int b) ' + virt_specifier + ';',
+ 'virtual void F(int a,',
+ ' int b) LOCKS_EXCLUDED(lock) ' + virt_specifier + ';',
+ 'virtual void F(int a,',
+ ' int b)',
+ ' LOCKS_EXCLUDED(lock) ' + virt_specifier + ';',
+ ''],
+ error_collector)
+ self.assertEquals(
+ [error_message, error_message, error_message],
+ error_collector.Results())
+
+ error_message = message_template % ('override', 'final')
+ self.TestLint('int F() override final', error_message)
+ self.TestLint('int F() override final;', error_message)
+ self.TestLint('int F() override final {}', error_message)
+ self.TestLint('int F() final override', error_message)
+ self.TestLint('int F() final override;', error_message)
+ self.TestLint('int F() final override {}', error_message)
+
def testCheckDeprecated(self):
self.TestLanguageRulesCheck('foo.cc', '#include <iostream>',
'Streams are highly discouraged.'
@@ -1423,11 +1539,10 @@
self.TestLint('delete a[some_var];', '')
self.TestLint('return a[some_var];', '')
- # DISALLOW_EVIL_CONSTRUCTORS should be at end of class if present.
- # Same with DISALLOW_COPY_AND_ASSIGN and DISALLOW_IMPLICIT_CONSTRUCTORS.
- def testDisallowEvilConstructors(self):
+ # DISALLOW_COPY_AND_ASSIGN and DISALLOW_IMPLICIT_CONSTRUCTORS should be at
+ # end of class if present.
+ def testDisallowMacrosAtEnd(self):
for macro_name in (
- 'DISALLOW_EVIL_CONSTRUCTORS',
'DISALLOW_COPY_AND_ASSIGN',
'DISALLOW_IMPLICIT_CONSTRUCTORS'):
self.TestLanguageRulesCheck(
@@ -1458,7 +1573,6 @@
# DISALLOW* macros should be in the private: section.
def testMisplacedDisallowMacros(self):
for macro_name in (
- 'DISALLOW_EVIL_CONSTRUCTORS',
'DISALLOW_COPY_AND_ASSIGN',
'DISALLOW_IMPLICIT_CONSTRUCTORS'):
self.TestMultiLineLint(
@@ -1507,17 +1621,17 @@
"""
class Outer3 {
struct Inner3 {
- DISALLOW_EVIL_CONSTRUCTORS(Inner3);
+ DISALLOW_COPY_AND_ASSIGN(Inner3);
};
DISALLOW_IMPLICIT_CONSTRUCTORS(Outer3);
};""",
- ('DISALLOW_EVIL_CONSTRUCTORS must be in the private: section'
+ ('DISALLOW_COPY_AND_ASSIGN must be in the private: section'
' [readability/constructors] [3]'))
self.TestMultiLineLint(
"""
struct Outer4 {
class Inner4 {
- DISALLOW_EVIL_CONSTRUCTORS(Inner4);
+ DISALLOW_COPY_AND_ASSIGN(Inner4);
};
DISALLOW_IMPLICIT_CONSTRUCTORS(Outer4);
};""",
@@ -1551,100 +1665,100 @@
# CHECK/EXPECT_TRUE/EXPECT_FALSE replacements
def testCheckCheck(self):
- self.TestLint('CHECK(x == 42)',
+ self.TestLint('CHECK(x == 42);',
'Consider using CHECK_EQ instead of CHECK(a == b)'
' [readability/check] [2]')
- self.TestLint('CHECK(x != 42)',
+ self.TestLint('CHECK(x != 42);',
'Consider using CHECK_NE instead of CHECK(a != b)'
' [readability/check] [2]')
- self.TestLint('CHECK(x >= 42)',
+ self.TestLint('CHECK(x >= 42);',
'Consider using CHECK_GE instead of CHECK(a >= b)'
' [readability/check] [2]')
- self.TestLint('CHECK(x > 42)',
+ self.TestLint('CHECK(x > 42);',
'Consider using CHECK_GT instead of CHECK(a > b)'
' [readability/check] [2]')
- self.TestLint('CHECK(x <= 42)',
+ self.TestLint('CHECK(x <= 42);',
'Consider using CHECK_LE instead of CHECK(a <= b)'
' [readability/check] [2]')
- self.TestLint('CHECK(x < 42)',
+ self.TestLint('CHECK(x < 42);',
'Consider using CHECK_LT instead of CHECK(a < b)'
' [readability/check] [2]')
- self.TestLint('DCHECK(x == 42)',
+ self.TestLint('DCHECK(x == 42);',
'Consider using DCHECK_EQ instead of DCHECK(a == b)'
' [readability/check] [2]')
- self.TestLint('DCHECK(x != 42)',
+ self.TestLint('DCHECK(x != 42);',
'Consider using DCHECK_NE instead of DCHECK(a != b)'
' [readability/check] [2]')
- self.TestLint('DCHECK(x >= 42)',
+ self.TestLint('DCHECK(x >= 42);',
'Consider using DCHECK_GE instead of DCHECK(a >= b)'
' [readability/check] [2]')
- self.TestLint('DCHECK(x > 42)',
+ self.TestLint('DCHECK(x > 42);',
'Consider using DCHECK_GT instead of DCHECK(a > b)'
' [readability/check] [2]')
- self.TestLint('DCHECK(x <= 42)',
+ self.TestLint('DCHECK(x <= 42);',
'Consider using DCHECK_LE instead of DCHECK(a <= b)'
' [readability/check] [2]')
- self.TestLint('DCHECK(x < 42)',
+ self.TestLint('DCHECK(x < 42);',
'Consider using DCHECK_LT instead of DCHECK(a < b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_TRUE("42" == x)',
+ 'EXPECT_TRUE("42" == x);',
'Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_TRUE("42" != x)',
+ 'EXPECT_TRUE("42" != x);',
'Consider using EXPECT_NE instead of EXPECT_TRUE(a != b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_TRUE(+42 >= x)',
+ 'EXPECT_TRUE(+42 >= x);',
'Consider using EXPECT_GE instead of EXPECT_TRUE(a >= b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_TRUE_M(-42 > x)',
+ 'EXPECT_TRUE_M(-42 > x);',
'Consider using EXPECT_GT_M instead of EXPECT_TRUE_M(a > b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_TRUE_M(42U <= x)',
+ 'EXPECT_TRUE_M(42U <= x);',
'Consider using EXPECT_LE_M instead of EXPECT_TRUE_M(a <= b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_TRUE_M(42L < x)',
+ 'EXPECT_TRUE_M(42L < x);',
'Consider using EXPECT_LT_M instead of EXPECT_TRUE_M(a < b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_FALSE(x == 42)',
+ 'EXPECT_FALSE(x == 42);',
'Consider using EXPECT_NE instead of EXPECT_FALSE(a == b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_FALSE(x != 42)',
+ 'EXPECT_FALSE(x != 42);',
'Consider using EXPECT_EQ instead of EXPECT_FALSE(a != b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_FALSE(x >= 42)',
+ 'EXPECT_FALSE(x >= 42);',
'Consider using EXPECT_LT instead of EXPECT_FALSE(a >= b)'
' [readability/check] [2]')
self.TestLint(
- 'ASSERT_FALSE(x > 42)',
+ 'ASSERT_FALSE(x > 42);',
'Consider using ASSERT_LE instead of ASSERT_FALSE(a > b)'
' [readability/check] [2]')
self.TestLint(
- 'ASSERT_FALSE(x <= 42)',
+ 'ASSERT_FALSE(x <= 42);',
'Consider using ASSERT_GT instead of ASSERT_FALSE(a <= b)'
' [readability/check] [2]')
self.TestLint(
- 'ASSERT_FALSE_M(x < 42)',
+ 'ASSERT_FALSE_M(x < 42);',
'Consider using ASSERT_GE_M instead of ASSERT_FALSE_M(a < b)'
' [readability/check] [2]')
- self.TestLint('CHECK(x<42)',
+ self.TestLint('CHECK(x<42);',
['Missing spaces around <'
' [whitespace/operators] [3]',
'Consider using CHECK_LT instead of CHECK(a < b)'
' [readability/check] [2]'])
- self.TestLint('CHECK(x>42)',
+ self.TestLint('CHECK(x>42);',
['Missing spaces around >'
' [whitespace/operators] [3]',
'Consider using CHECK_GT instead of CHECK(a > b)'
@@ -1653,41 +1767,42 @@
self.TestLint('using some::namespace::operator<<;', '')
self.TestLint('using some::namespace::operator>>;', '')
- self.TestLint('CHECK(x->y == 42)',
+ self.TestLint('CHECK(x->y == 42);',
'Consider using CHECK_EQ instead of CHECK(a == b)'
' [readability/check] [2]')
self.TestLint(
- ' EXPECT_TRUE(42 < x) // Random comment.',
+ ' EXPECT_TRUE(42 < x); // Random comment.',
'Consider using EXPECT_LT instead of EXPECT_TRUE(a < b)'
' [readability/check] [2]')
self.TestLint(
- 'EXPECT_TRUE( 42 < x )',
+ 'EXPECT_TRUE( 42 < x );',
['Extra space after ( in function call'
' [whitespace/parens] [4]',
+ 'Extra space before ) [whitespace/parens] [2]',
'Consider using EXPECT_LT instead of EXPECT_TRUE(a < b)'
' [readability/check] [2]'])
- self.TestLint('CHECK(4\'2 == x)',
+ self.TestLint('CHECK(4\'2 == x);',
'Consider using CHECK_EQ instead of CHECK(a == b)'
' [readability/check] [2]')
def testCheckCheckFalsePositives(self):
- self.TestLint('CHECK(some_iterator == obj.end())', '')
- self.TestLint('EXPECT_TRUE(some_iterator == obj.end())', '')
- self.TestLint('EXPECT_FALSE(some_iterator == obj.end())', '')
- self.TestLint('CHECK(some_pointer != NULL)', '')
- self.TestLint('EXPECT_TRUE(some_pointer != NULL)', '')
- self.TestLint('EXPECT_FALSE(some_pointer != NULL)', '')
+ self.TestLint('CHECK(some_iterator == obj.end());', '')
+ self.TestLint('EXPECT_TRUE(some_iterator == obj.end());', '')
+ self.TestLint('EXPECT_FALSE(some_iterator == obj.end());', '')
+ self.TestLint('CHECK(some_pointer != NULL);', '')
+ self.TestLint('EXPECT_TRUE(some_pointer != NULL);', '')
+ self.TestLint('EXPECT_FALSE(some_pointer != NULL);', '')
self.TestLint('CHECK(CreateTestFile(dir, (1 << 20)));', '')
self.TestLint('CHECK(CreateTestFile(dir, (1 >> 20)));', '')
- self.TestLint('CHECK(x ^ (y < 42))', '')
- self.TestLint('CHECK((x > 42) ^ (x < 54))', '')
- self.TestLint('CHECK(a && b < 42)', '')
- self.TestLint('CHECK(42 < a && a < b)', '')
- self.TestLint('SOFT_CHECK(x > 42)', '')
+ self.TestLint('CHECK(x ^ (y < 42));', '')
+ self.TestLint('CHECK((x > 42) ^ (x < 54));', '')
+ self.TestLint('CHECK(a && b < 42);', '')
+ self.TestLint('CHECK(42 < a && a < b);', '')
+ self.TestLint('SOFT_CHECK(x > 42);', '')
self.TestMultiLineLint(
"""_STLP_DEFINE_BINARY_OP_CHECK(==, _OP_EQUAL);
@@ -1703,6 +1818,8 @@
_STLP_DEFINE_BINARY_OP_CHECK(%, _OP_MOD);""",
'')
+ self.TestLint('CHECK(x < 42) << "Custom error message";', '')
+
# Alternative token to punctuation operator replacements
def testCheckAltTokens(self):
self.TestLint('true or true',
@@ -1822,12 +1939,20 @@
'vector <int> &nonconst_x) {',
operand_error_message % 'vector<int> &nonconst_x')
+ # Derived member functions are spared from override check
+ self.TestLint('void Func(X& x);', operand_error_message % 'X& x')
+ self.TestLint('void Func(X& x) {}', operand_error_message % 'X& x')
+ self.TestLint('void Func(X& x) override;', '')
+ self.TestLint('void Func(X& x) override {', '')
+ self.TestLint('void Func(X& x) const override;', '')
+ self.TestLint('void Func(X& x) const override {', '')
+
# Other potential false positives. These need full parser
# state to reproduce as opposed to just TestLint.
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData(
'foo.cc', 'cc',
- ['// Copyright 2008 Your Company. All Rights Reserved.',
+ ['// Copyright 2014 Your Company. All Rights Reserved.',
'void swap(int &x,',
' int &y) {',
'}',
@@ -1839,18 +1964,24 @@
' ostream& out',
' const dense_hash_set<Value, Hash, Equals, Alloc>& seq) {',
'}',
+ 'void Derived::Function(',
+ ' string &x) override {',
+ '}',
'#define UNSUPPORTED_MASK(_mask) \\',
' if (flags & _mask) { \\',
' LOG(FATAL) << "Unsupported flag: " << #_mask; \\',
' }',
'Constructor::Constructor()',
- ' : initializer1_(a & b),',
- ' initializer2_(a & b) {',
+ ' : initializer1_(a1 & b1),',
+ ' initializer2_(a2 & b2) {',
'}',
'Constructor::Constructor()',
- ' : initializer1_{a & b},',
- ' initializer2_(a & b) {',
+ ' : initializer1_{a3 & b3},',
+ ' initializer2_(a4 & b4) {',
'}',
+ 'Constructor::Constructor()',
+ ' : initializer1_{a5 & b5},',
+ ' initializer2_(a6 & b6) {}',
''],
error_collector)
self.assertEquals('', error_collector.Results())
@@ -1859,7 +1990,7 @@
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData(
'foo.cc', 'cc',
- ['// Copyright 2008 Your Company. All Rights Reserved.',
+ ['// Copyright 2014 Your Company. All Rights Reserved.',
'void Func(const Outer::',
' Inner& const_x,',
' const Outer',
@@ -1885,7 +2016,7 @@
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData(
'foo.cc', 'cc',
- ['// Copyright 2008 Your Company. All Rights Reserved.',
+ ['// Copyright 2014 Your Company. All Rights Reserved.',
'inline RCULocked<X>::ReadPtr::ReadPtr(const RCULocked* rcu) {',
' DCHECK(!(data & kFlagMask)) << "Error";',
'}',
@@ -2246,10 +2377,11 @@
self.TestLint('A(a b) : c(d&& e)', space_error)
self.TestLint('A(a b) : c(), d(e&& f)', space_error)
+ def testAllowedRvalueReference(self):
# Verify that RValue reference warnings for a line range can be silenced
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData('foo.cc', 'cc',
- ['// Copyright 2008 Your Company.',
+ ['// Copyright 2014 Your Company.',
'GOOGLE_ALLOW_RVALUE_REFERENCES_PUSH',
'void F(A&& b);',
'GOOGLE_ALLOW_RVALUE_REFERENCES_POP',
@@ -2257,6 +2389,22 @@
error_collector)
self.assertEquals(error_collector.Results(), '')
+ # RValue references for constructors and operator=
+ error_collector = ErrorCollector(self.assert_)
+ cpplint.ProcessFileData(
+ 'foo.cc', 'cc',
+ ['// Copyright 2014 Your Company.',
+ 'class X {',
+ ' X(X&& param) = delete; // NOLINT(runtime/explicit)',
+ ' X(X &¶m) = default; // NOLINT(runtime/explicit)',
+ '',
+ ' X& operator=(X&& param) = delete;',
+ ' X& operator=(X&& param)=default;',
+ '};',
+ ''],
+ error_collector)
+ self.assertEquals(error_collector.Results(), '')
+
def testSpacingBeforeLastSemicolon(self):
self.TestLint('call_function() ;',
'Extra space before last semicolon. If this should be an '
@@ -2334,26 +2482,19 @@
# Static or global STL strings.
def testStaticOrGlobalSTLStrings(self):
+ error_msg = ('For a static/global string constant, use a C style '
+ 'string instead: "%s[]". [runtime/string] [4]')
+
self.TestLint('string foo;',
- 'For a static/global string constant, use a C style '
- 'string instead: "char foo[]".'
- ' [runtime/string] [4]')
+ error_msg % 'char foo')
self.TestLint('string kFoo = "hello"; // English',
- 'For a static/global string constant, use a C style '
- 'string instead: "char kFoo[]".'
- ' [runtime/string] [4]')
+ error_msg % 'char kFoo')
self.TestLint('static string foo;',
- 'For a static/global string constant, use a C style '
- 'string instead: "static char foo[]".'
- ' [runtime/string] [4]')
+ error_msg % 'static char foo')
self.TestLint('static const string foo;',
- 'For a static/global string constant, use a C style '
- 'string instead: "static const char foo[]".'
- ' [runtime/string] [4]')
+ error_msg % 'static const char foo')
self.TestLint('string Foo::bar;',
- 'For a static/global string constant, use a C style '
- 'string instead: "char Foo::bar[]".'
- ' [runtime/string] [4]')
+ error_msg % 'char Foo::bar')
self.TestLint('string* pointer', '')
self.TestLint('string *pointer', '')
self.TestLint('string* pointer = Func();', '')
@@ -2373,10 +2514,7 @@
self.TestLint('string Foo::bar() {}', '')
self.TestLint('string Foo::operator*() {}', '')
# Rare case.
- self.TestLint('string foo("foobar");',
- 'For a static/global string constant, use a C style '
- 'string instead: "char foo[]".'
- ' [runtime/string] [4]')
+ self.TestLint('string foo("foobar");', error_msg % 'char foo')
# Should not catch local or member variables.
self.TestLint(' string foo', '')
# Should not catch functions.
@@ -2400,6 +2538,34 @@
' return "";\n'
'}\n', '')
+ # Check multiline cases.
+ error_collector = ErrorCollector(self.assert_)
+ cpplint.ProcessFileData('foo.cc', 'cc',
+ ['// Copyright 2014 Your Company.',
+ 'string Class',
+ '::MemberFunction1();',
+ 'string Class::',
+ 'MemberFunction2();',
+ 'string Class::',
+ 'NestedClass::MemberFunction3();',
+ 'string TemplateClass<T>::',
+ 'NestedClass::MemberFunction4();',
+ 'string Class',
+ '::static_member_variable1;',
+ 'string Class::',
+ 'static_member_variable2;',
+ 'string Class',
+ '::static_member_variable3 = "initial value";',
+ 'string Class::',
+ 'static_member_variable4 = "initial value";',
+ ''],
+ error_collector)
+ self.assertEquals(error_collector.Results(),
+ [error_msg % 'char Class::static_member_variable1',
+ error_msg % 'char Class::static_member_variable2',
+ error_msg % 'char Class::static_member_variable3',
+ error_msg % 'char Class::static_member_variable4'])
+
def testNoSpacesInFunctionCalls(self):
self.TestLint('TellStory(1, 3);',
'')
@@ -2571,7 +2737,7 @@
# Test for NUL bytes only
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData('nul.cc', 'cc',
- ['// Copyright 2008 Your Company.',
+ ['// Copyright 2014 Your Company.',
'\0', ''], error_collector)
self.assertEquals(
error_collector.Results(),
@@ -2582,7 +2748,7 @@
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData(
'nul_utf8.cc', 'cc',
- ['// Copyright 2008 Your Company.',
+ ['// Copyright 2014 Your Company.',
unicode('\xe9x\0', 'utf8', 'replace'), ''],
error_collector)
self.assertEquals(
@@ -2605,6 +2771,14 @@
['\n', '// {\n', '\n', '\n', '// Comment\n', '{\n', '}\n'], 0, 0)
self.TestBlankLinesCheck(['\n', 'run("{");\n', '\n'], 0, 0)
self.TestBlankLinesCheck(['\n', ' if (foo) { return 0; }\n', '\n'], 0, 0)
+ self.TestBlankLinesCheck(
+ ['int x(\n', ' int a) {\n', '\n', 'return 0;\n', '}'], 0, 0)
+ self.TestBlankLinesCheck(
+ ['int x(\n', ' int a) const {\n', '\n', 'return 0;\n', '}'], 0, 0)
+ self.TestBlankLinesCheck(
+ ['int x(\n', ' int a) {\n', '\n', 'return 0;\n', '}'], 1, 0)
+ self.TestBlankLinesCheck(
+ ['int x(\n', ' int a) {\n', '\n', 'return 0;\n', '}'], 1, 0)
def testAllowBlankLineBeforeClosingNamespace(self):
error_collector = ErrorCollector(self.assert_)
@@ -2727,7 +2901,7 @@
def testAllowBlankLinesInRawStrings(self):
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData('foo.cc', 'cc',
- ['// Copyright 2008 Your Company.',
+ ['// Copyright 2014 Your Company.',
'static const char *kData[] = {R"(',
'',
')", R"(',
@@ -2885,6 +3059,9 @@
self.TestLint('f(a, /* name */"1");', '')
self.TestLint('f(1, /* empty macro arg */, 2)', '')
self.TestLint('f(1,, 2)', '')
+ self.TestLint('operator,()', '')
+ self.TestLint('operator,(a,b)',
+ 'Missing space after , [whitespace/comma] [3]')
def testIndent(self):
self.TestLint('static int noindent;', '')
@@ -2899,6 +3076,20 @@
self.TestLint(' char* one_space_indent = "public:";',
'Weird number of spaces at line-start. '
'Are you using a 2-space indent? [whitespace/indent] [3]')
+ self.TestLint(' public:', '')
+ self.TestLint(' protected:', '')
+ self.TestLint(' private:', '')
+ self.TestLint(' protected: \\', '')
+ self.TestLint(' public: \\', '')
+ self.TestLint(' private: \\', '')
+ self.TestMultiLineLint(
+ TrimExtraIndent("""
+ class foo {
+ public slots:
+ void bar();
+ };"""),
+ 'Weird number of spaces at line-start. '
+ 'Are you using a 2-space indent? [whitespace/indent] [3]')
self.TestMultiLineLint(
TrimExtraIndent('''
static const char kRawString[] = R"("
@@ -3405,6 +3596,14 @@
# Make sure we extracted something for our header guard.
self.assertNotEqual(expected_guard, '')
+ # No header guard, but the error is suppressed.
+ error_collector = ErrorCollector(self.assert_)
+ cpplint.ProcessFileData(file_path, 'h',
+ ['// Copyright 2014 Your Company.',
+ '// NOLINT(build/header_guard)', ''],
+ error_collector)
+ self.assertEquals([], error_collector.ResultList())
+
# Wrong guard
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData(file_path, 'h',
@@ -3601,6 +3800,8 @@
self.TestLint('#include "foo.h"',
'Include the directory when naming .h files'
' [build/include] [4]')
+ self.TestLint('#include "Python.h"', '')
+ self.TestLint('#include "lua.h"', '')
def testBuildPrintfFormat(self):
error_collector = ErrorCollector(self.assert_)
@@ -3620,7 +3821,7 @@
error_collector = ErrorCollector(self.assert_)
cpplint.ProcessFileData(
'foo.cc', 'cc',
- ['// Copyright 2008 Your Company.',
+ ['// Copyright 2014 Your Company.',
r'printf("\\%%%d", value);',
r'printf(R"(\[)");',
r'printf(R"(\[%s)", R"(\])");',
@@ -3731,7 +3932,7 @@
'You should have a line: "Copyright [year] <Copyright Owner>"'
' [legal/copyright] [5]')
- copyright_line = '// Copyright 2008 Google Inc. All Rights Reserved.'
+ copyright_line = '// Copyright 2014 Google Inc. All Rights Reserved.'
file_path = 'mydir/googleclient/foo.cc'
@@ -3930,14 +4131,10 @@
class OrderOfIncludesTest(CpplintTestBase):
def setUp(self):
+ CpplintTestBase.setUp(self)
self.include_state = cpplint._IncludeState()
- # Cheat os.path.abspath called in FileInfo class.
- self.os_path_abspath_orig = os.path.abspath
os.path.abspath = lambda value: value
- def tearDown(self):
- os.path.abspath = self.os_path_abspath_orig
-
def testCheckNextIncludeOrder_OtherThenCpp(self):
self.assertEqual('', self.include_state.CheckNextIncludeOrder(
cpplint._OTHER_HEADER))
@@ -4154,6 +4351,11 @@
'Should be: a.h, c system, c++ system, other. '
'[build/include_order] [4]'))
+ # Third party headers are exempt from order checks
+ self.TestLanguageRulesCheck('foo/foo.cc',
+ Format(['<string>', '"Python.h"', '<vector>']),
+ '')
+
class CheckForFunctionLengthsTest(CpplintTestBase):
@@ -4460,7 +4662,7 @@
self.TestFunctionLengthsCheck(
'MACRO_WITH_UNDERSCORES(arg1, arg2, arg3)',
'')
-
+
self.TestFunctionLengthsCheck(
'NonMacro(arg)',
'Lint failed to find start of function body.'
@@ -4942,7 +5144,6 @@
if __name__ == '__main__':
- import sys
# We don't want to run the VerifyAllCategoriesAreSeen() test unless
# we're running the full test suite: if we only run one test,
# obviously we're not going to see all the error categories. So we