diff --git a/cpplint/cpplint.py b/cpplint/cpplint.py index 61bae457e..94913206f 100755 --- a/cpplint/cpplint.py +++ b/cpplint/cpplint.py @@ -177,6 +177,8 @@ _ERROR_CATEGORIES = [ 'build/class', 'build/c++11', + 'build/c++14', + 'build/c++tr1', 'build/deprecated', 'build/endif_comment', 'build/explicit_make_pair', @@ -196,7 +198,6 @@ 'readability/check', 'readability/constructors', 'readability/fn_size', - 'readability/function', 'readability/inheritance', 'readability/multiline_comment', 'readability/multiline_string', @@ -227,6 +228,7 @@ 'whitespace/comma', 'whitespace/comments', 'whitespace/empty_conditional_body', + 'whitespace/empty_if_body', 'whitespace/empty_loop_body', 'whitespace/end_of_line', 'whitespace/ending_newline', @@ -245,6 +247,7 @@ # compatibility they may still appear in NOLINT comments. _LEGACY_ERROR_CATEGORIES = [ 'readability/streams', + 'readability/function', ] # The default state of the category filter. This is overridden by the --filter= @@ -253,6 +256,16 @@ # All entries here should start with a '-' or '+', as in the --filter= flag. _DEFAULT_FILTERS = ['-build/include_alpha'] +# The default list of categories suppressed for C (not C++) files. +_DEFAULT_C_SUPPRESSED_CATEGORIES = [ + 'readability/casting', + ] + +# The default list of categories suppressed for Linux Kernel files. +_DEFAULT_KERNEL_SUPPRESSED_CATEGORIES = [ + 'whitespace/tab', + ] + # We used to check for high-bit characters, but after much discussion we # decided those were OK, as long as they were in UTF-8 and didn't represent # hard-coded international strings, which belong in a separate i18n file. @@ -346,6 +359,7 @@ 'random', 'ratio', 'regex', + 'scoped_allocator', 'set', 'sstream', 'stack', @@ -393,6 +407,19 @@ 'cwctype', ]) +# Type names +_TYPES = re.compile( + r'^(?:' + # [dcl.type.simple] + r'(char(16_t|32_t)?)|wchar_t|' + r'bool|short|int|long|signed|unsigned|float|double|' + # [support.types] + r'(ptrdiff_t|size_t|max_align_t|nullptr_t)|' + # [cstdint.syn] + r'(u?int(_fast|_least)?(8|16|32|64)_t)|' + r'(u?int(max|ptr)_t)|' + r')$') + # These headers are excluded from [build/include] and [build/include_order] # checks: @@ -402,16 +429,18 @@ _THIRD_PARTY_HEADERS_PATTERN = re.compile( r'^(?:[^/]*[A-Z][^/]*\.h|lua\.h|lauxlib\.h|lualib\.h)$') +# Pattern for matching FileInfo.BaseName() against test file name +_TEST_FILE_SUFFIX = r'(_test|_unittest|_regtest)$' + +# Pattern that matches only complete whitespace, possibly across multiple lines. +_EMPTY_CONDITIONAL_BODY_PATTERN = re.compile(r'^\s*$', re.DOTALL) # 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. +# testing/base/public/gunit.h. _CHECK_MACROS = [ 'DCHECK', 'CHECK', - 'EXPECT_TRUE_M', 'EXPECT_TRUE', - 'ASSERT_TRUE_M', 'ASSERT_TRUE', - 'EXPECT_FALSE_M', 'EXPECT_FALSE', - 'ASSERT_FALSE_M', 'ASSERT_FALSE', + 'EXPECT_TRUE', 'ASSERT_TRUE', + 'EXPECT_FALSE', 'ASSERT_FALSE', ] # Replacement macros for CHECK/DCHECK/EXPECT_TRUE/EXPECT_FALSE @@ -424,16 +453,12 @@ _CHECK_REPLACEMENT['CHECK'][op] = 'CHECK_%s' % replacement _CHECK_REPLACEMENT['EXPECT_TRUE'][op] = 'EXPECT_%s' % replacement _CHECK_REPLACEMENT['ASSERT_TRUE'][op] = 'ASSERT_%s' % replacement - _CHECK_REPLACEMENT['EXPECT_TRUE_M'][op] = 'EXPECT_%s_M' % replacement - _CHECK_REPLACEMENT['ASSERT_TRUE_M'][op] = 'ASSERT_%s_M' % replacement for op, inv_replacement in [('==', 'NE'), ('!=', 'EQ'), ('>=', 'LT'), ('>', 'LE'), ('<=', 'GT'), ('<', 'GE')]: _CHECK_REPLACEMENT['EXPECT_FALSE'][op] = 'EXPECT_%s' % inv_replacement _CHECK_REPLACEMENT['ASSERT_FALSE'][op] = 'ASSERT_%s' % inv_replacement - _CHECK_REPLACEMENT['EXPECT_FALSE_M'][op] = 'EXPECT_%s_M' % inv_replacement - _CHECK_REPLACEMENT['ASSERT_FALSE_M'][op] = 'ASSERT_%s_M' % inv_replacement # Alternative tokens and their replacements. For full list, see section 2.5 # Alternative tokens [lex.digraph] in the C++ standard. @@ -482,6 +507,12 @@ r'(?:\s+(volatile|__volatile__))?' r'\s*[{(]') +# Match strings that indicate we're working on a C (not C++) file. +_SEARCH_C_FILE = re.compile(r'\b(?:LINT_C_FILE|' + r'vim?:\s*.*(\s*|:)filetype=c(\s*|:|$))') + +# Match string that indicates we're working on a Linux Kernel file. +_SEARCH_KERNEL_FILE = re.compile(r'\b(?:LINT_KERNEL_FILE)') _regexp_compile_cache = {} @@ -501,8 +532,13 @@ # This is set by --extensions flag. _valid_extensions = set(['cc', 'h', 'cpp', 'cu', 'cuh']) +# {str, bool}: a map from error categories to booleans which indicate if the +# category should be suppressed for every line. +_global_error_suppressions = {} + + def ParseNolintSuppressions(filename, raw_line, linenum, error): - """Updates the global list of error-suppressions. + """Updates the global list of line error-suppressions. Parses any NOLINT comments on the current line, updating the global error_suppressions store. Reports an error if the NOLINT comment @@ -533,24 +569,45 @@ def ParseNolintSuppressions(filename, raw_line, linenum, error): 'Unknown NOLINT error category: %s' % category) +def ProcessGlobalSuppresions(lines): + """Updates the list of global error suppressions. + + Parses any lint directives in the file that have global effect. + + Args: + lines: An array of strings, each representing a line of the file, with the + last element being empty if the file is terminated with a newline. + """ + for line in lines: + if _SEARCH_C_FILE.search(line): + for category in _DEFAULT_C_SUPPRESSED_CATEGORIES: + _global_error_suppressions[category] = True + if _SEARCH_KERNEL_FILE.search(line): + for category in _DEFAULT_KERNEL_SUPPRESSED_CATEGORIES: + _global_error_suppressions[category] = True + + def ResetNolintSuppressions(): """Resets the set of NOLINT suppressions to empty.""" _error_suppressions.clear() + _global_error_suppressions.clear() def IsErrorSuppressedByNolint(category, linenum): """Returns true if the specified error category is suppressed on this line. Consults the global error_suppressions map populated by - ParseNolintSuppressions/ResetNolintSuppressions. + ParseNolintSuppressions/ProcessGlobalSuppresions/ResetNolintSuppressions. Args: category: str, the category of the error. linenum: int, the current line number. Returns: - bool, True iff the error should be suppressed due to a NOLINT comment. + bool, True iff the error should be suppressed due to a NOLINT comment or + global suppression. """ - return (linenum in _error_suppressions.get(category, set()) or + return (_global_error_suppressions.get(category, False) or + linenum in _error_suppressions.get(category, set()) or linenum in _error_suppressions.get(None, set())) @@ -589,6 +646,11 @@ def Search(pattern, s): return _regexp_compile_cache[pattern].search(s) +def _IsSourceExtension(s): + """File extension (excluding dot) matches a source file extension.""" + return s in ('c', 'cc', 'cpp', 'cxx') + + class _IncludeState(object): """Tracks line numbers for includes, and the order in which includes appear. @@ -944,6 +1006,9 @@ def Check(self, error, filename, linenum): filename: The name of the current file. linenum: The number of the line to check. """ + if not self.in_a_function: + return + if Match(r'T(EST|est)', self.current_function): base_trigger = self._TEST_TRIGGER else: @@ -1014,12 +1079,13 @@ def RepositoryName(self): # Not SVN <= 1.6? Try to find a git, hg, or svn top level directory by # searching up from the current path. - root_dir = os.path.dirname(fullname) - while (root_dir != os.path.dirname(root_dir) and - not os.path.exists(os.path.join(root_dir, ".git")) and - not os.path.exists(os.path.join(root_dir, ".hg")) and - not os.path.exists(os.path.join(root_dir, ".svn"))): - root_dir = os.path.dirname(root_dir) + root_dir = current_dir = os.path.dirname(fullname) + while current_dir != os.path.dirname(current_dir): + if (os.path.exists(os.path.join(current_dir, ".git")) or + os.path.exists(os.path.join(current_dir, ".hg")) or + os.path.exists(os.path.join(current_dir, ".svn"))): + root_dir = current_dir + current_dir = os.path.dirname(current_dir) if (os.path.exists(os.path.join(root_dir, ".git")) or os.path.exists(os.path.join(root_dir, ".hg")) or @@ -1058,7 +1124,7 @@ def NoExtension(self): def IsSource(self): """File has a source file extension.""" - return self.Extension()[1:] in ('c', 'cc', 'cpp', 'cxx') + return _IsSourceExtension(self.Extension()[1:]) def _ShouldPrintError(category, confidence, linenum): @@ -1204,8 +1270,18 @@ def CleanseRawStrings(raw_lines): while delimiter is None: # Look for beginning of a raw string. # See 2.14.15 [lex.string] for syntax. - matched = Match(r'^(.*)\b(?:R|u8R|uR|UR|LR)"([^\s\\()]*)\((.*)$', line) - if matched: + # + # Once we have matched a raw string, we check the prefix of the + # line to make sure that the line is not part of a single line + # comment. It's done this way because we remove raw strings + # before removing comments as opposed to removing comments + # before removing raw strings. This is because there are some + # cpplint checks that requires the comments to be preserved, but + # we don't want to check comments that are inside raw strings. + matched = Match(r'^(.*?)\b(?:R|u8R|uR|UR|LR)"([^\s\\()]*)\((.*)$', line) + if (matched and + not Match(r'^([^\'"]|\'(\\.|[^\'])*\'|"(\\.|[^"])*")*//', + matched.group(1))): delimiter = ')' + matched.group(2) + '"' end = matched.group(3).find(delimiter) @@ -1666,7 +1742,7 @@ def GetHeaderGuardCPPVariable(filename): filename = re.sub(r'/\.flymake/([^/]*)$', r'/\1', filename) # Replace 'c++' with 'cpp'. filename = filename.replace('C++', 'cpp').replace('c++', 'cpp') - + fileinfo = FileInfo(filename) file_path_from_root = fileinfo.RepositoryName() if _root: @@ -1776,11 +1852,11 @@ def CheckHeaderFileIncluded(filename, include_state, error): """Logs an error if a .cc file does not include its header.""" # Do not check test files - if filename.endswith('_test.cc') or filename.endswith('_unittest.cc'): + fileinfo = FileInfo(filename) + if Search(_TEST_FILE_SUFFIX, fileinfo.BaseName()): return - fileinfo = FileInfo(filename) - headerfile = filename[0:len(filename) - 2] + 'h' + headerfile = filename[0:len(filename) - len(fileinfo.Extension())] + '.h' if not os.path.exists(headerfile): return headername = FileInfo(headerfile).RepositoryName() @@ -1997,7 +2073,8 @@ def IsForwardClassDeclaration(clean_lines, linenum): class _BlockInfo(object): """Stores information about a generic block of code.""" - def __init__(self, seen_open_brace): + def __init__(self, linenum, seen_open_brace): + self.starting_linenum = linenum self.seen_open_brace = seen_open_brace self.open_parentheses = 0 self.inline_asm = _NO_ASM @@ -2046,17 +2123,16 @@ def IsBlockInfo(self): class _ExternCInfo(_BlockInfo): """Stores information about an 'extern "C"' block.""" - def __init__(self): - _BlockInfo.__init__(self, True) + def __init__(self, linenum): + _BlockInfo.__init__(self, linenum, True) class _ClassInfo(_BlockInfo): """Stores information about a class.""" def __init__(self, name, class_or_struct, clean_lines, linenum): - _BlockInfo.__init__(self, False) + _BlockInfo.__init__(self, linenum, False) self.name = name - self.starting_linenum = linenum self.is_derived = False self.check_namespace_indentation = True if class_or_struct == 'struct': @@ -2124,9 +2200,8 @@ class _NamespaceInfo(_BlockInfo): """Stores information about a namespace.""" def __init__(self, name, linenum): - _BlockInfo.__init__(self, False) + _BlockInfo.__init__(self, linenum, False) self.name = name or '' - self.starting_linenum = linenum self.check_namespace_indentation = True def CheckEnd(self, filename, clean_lines, linenum, error): @@ -2145,7 +2220,7 @@ def CheckEnd(self, filename, clean_lines, linenum, error): # deciding what these nontrivial things are, so this check is # triggered by namespace size only, which works most of the time. if (linenum - self.starting_linenum < 10 - and not Match(r'};*\s*(//|/\*).*\bnamespace\b', line)): + and not Match(r'^\s*};*\s*(//|/\*).*\bnamespace\b', line)): return # Look for matching comment at end of namespace. @@ -2162,18 +2237,18 @@ def CheckEnd(self, filename, clean_lines, linenum, error): # expected namespace. if self.name: # Named namespace - if not Match((r'};*\s*(//|/\*).*\bnamespace\s+' + re.escape(self.name) + - r'[\*/\.\\\s]*$'), + if not Match((r'^\s*};*\s*(//|/\*).*\bnamespace\s+' + + re.escape(self.name) + r'[\*/\.\\\s]*$'), line): error(filename, linenum, 'readability/namespace', 5, 'Namespace should be terminated with "// namespace %s"' % self.name) else: # Anonymous namespace - if not Match(r'};*\s*(//|/\*).*\bnamespace[\*/\.\\\s]*$', line): + if not Match(r'^\s*};*\s*(//|/\*).*\bnamespace[\*/\.\\\s]*$', line): # If "// namespace anonymous" or "// anonymous namespace (more text)", # mention "// anonymous namespace" as an acceptable form - if Match(r'}.*\b(namespace anonymous|anonymous namespace)\b', line): + if Match(r'^\s*}.*\b(namespace anonymous|anonymous namespace)\b', line): error(filename, linenum, 'readability/namespace', 5, 'Anonymous namespace should be terminated with "// namespace"' ' or "// anonymous namespace"') @@ -2512,9 +2587,9 @@ def Update(self, filename, clean_lines, linenum, error): if not self.SeenOpenBrace(): self.stack[-1].seen_open_brace = True elif Match(r'^extern\s*"[^"]*"\s*\{', line): - self.stack.append(_ExternCInfo()) + self.stack.append(_ExternCInfo(linenum)) else: - self.stack.append(_BlockInfo(True)) + self.stack.append(_BlockInfo(linenum, True)) if _MATCH_ASM.match(line): self.stack[-1].inline_asm = _BLOCK_ASM @@ -2626,7 +2701,8 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, r'\s+(register|static|extern|typedef)\b', line): error(filename, linenum, 'build/storage_class', 5, - 'Storage class (static, extern, typedef, etc) should be first.') + 'Storage-class specifier (static, extern, typedef, etc) should be ' + 'at the beginning of the declaration.') if Match(r'\s*#\s*endif\s*[^/\s]+', line): error(filename, linenum, 'build/endif_comment', 5, @@ -2665,9 +2741,7 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, base_classname = classinfo.name.split('::')[-1] # Look for single-argument constructors that aren't 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. + # Technically a valid construct, but against style. explicit_constructor_match = Match( r'\s+(?:inline\s+)?(explicit\s+)?(?:inline\s+)?%s\s*' r'\(((?:[^()]|\([^()]*\))*)\)' @@ -2728,10 +2802,6 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, 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): @@ -2786,6 +2856,7 @@ def CheckSpacingForFunctionCall(filename, clean_lines, linenum, error): error(filename, linenum, 'whitespace/parens', 2, 'Extra space after (') if (Search(r'\w\s+\(', fncall) and + not Search(r'_{0,2}asm_{0,2}\s+_{0,2}volatile_{0,2}\s+\(', fncall) and not Search(r'#\s*define|typedef|using\s+\w+\s*=', fncall) and not Search(r'\w\s+\((\w+::)*\*\w+\)\(', fncall) and not Search(r'\bcase\s+\(', fncall)): @@ -2923,9 +2994,7 @@ def CheckComment(line, filename, linenum, next_line_start, error): commentpos = line.find('//') if commentpos != -1: # Check if the // may be in quotes. If so, ignore it - # Comparisons made explicit for clarity -- pylint: disable=g-explicit-bool-comparison - if (line.count('"', 0, commentpos) - - line.count('\\"', 0, commentpos)) % 2 == 0: # not in quotes + if re.sub(r'\\.', '', line[0:commentpos]).count('"') % 2 == 0: # Allow one space for new scopes, two spaces otherwise: if (not (Match(r'^.*{ *//', line) and next_line_start == commentpos) and ((commentpos >= 1 and @@ -3174,8 +3243,8 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # macro context and don't do any checks. This avoids false # positives. # - # Note that && is not included here. Those are checked separately - # in CheckRValueReference + # Note that && is not included here. This is because there are too + # many false positives due to RValue references. match = Search(r'[^<>=!\s](==|!=|<=|>=|\|\|)[^<>=!\s,;\)]', line) if match: error(filename, linenum, 'whitespace/operators', 3, @@ -3209,7 +3278,7 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # # 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) + match = Search(r'(operator|[^\s(<])(?:L|UL|LL|ULL|l|ul|ll|ull)?<<([^\s,=<])', line) if (match and not (match.group(1).isdigit() and match.group(2).isdigit()) and not (match.group(1) == 'operator' and match.group(2) == ';')): error(filename, linenum, 'whitespace/operators', 3, @@ -3313,22 +3382,90 @@ def CheckCommaSpacing(filename, clean_lines, linenum, error): 'Missing space after ;') -def CheckBracesSpacing(filename, clean_lines, linenum, error): +def _IsType(clean_lines, nesting_state, expr): + """Check if expression looks like a type name, returns true if so. + + Args: + clean_lines: A CleansedLines instance containing the file. + nesting_state: A NestingState instance which maintains information about + the current stack of nested blocks being parsed. + expr: The expression to check. + Returns: + True, if token looks like a type. + """ + # Keep only the last token in the expression + last_word = Match(r'^.*(\b\S+)$', expr) + if last_word: + token = last_word.group(1) + else: + token = expr + + # Match native types and stdint types + if _TYPES.match(token): + return True + + # Try a bit harder to match templated types. Walk up the nesting + # stack until we find something that resembles a typename + # declaration for what we are looking for. + typename_pattern = (r'\b(?:typename|class|struct)\s+' + re.escape(token) + + r'\b') + block_index = len(nesting_state.stack) - 1 + while block_index >= 0: + if isinstance(nesting_state.stack[block_index], _NamespaceInfo): + return False + + # Found where the opening brace is. We want to scan from this + # line up to the beginning of the function, minus a few lines. + # template + # class C + # : public ... { // start scanning here + last_line = nesting_state.stack[block_index].starting_linenum + + next_block_start = 0 + if block_index > 0: + next_block_start = nesting_state.stack[block_index - 1].starting_linenum + first_line = last_line + while first_line >= next_block_start: + if clean_lines.elided[first_line].find('template') >= 0: + break + first_line -= 1 + if first_line < next_block_start: + # Didn't find any "template" keyword before reaching the next block, + # there are probably no template things to check for this block + block_index -= 1 + continue + + # Look for typename in the specified range + for i in xrange(first_line, last_line + 1, 1): + if Search(typename_pattern, clean_lines.elided[i]): + return True + block_index -= 1 + + return False + + +def CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error): """Checks for horizontal spacing near commas. Args: filename: The name of the current file. clean_lines: A CleansedLines instance containing the file. linenum: The number of the line to check. + nesting_state: A NestingState instance which maintains information about + the current stack of nested blocks being parsed. error: The function to call with any errors found. """ line = clean_lines.elided[linenum] # Except after an opening paren, or after another opening brace (in case of # an initializer list, for instance), you should have spaces before your - # braces. And since you should never have braces at the beginning of a line, - # this is an easy test. + # braces when they are delimiting blocks, classes, namespaces etc. + # And since you should never have braces at the beginning of a line, + # this is an easy test. Except that braces used for initialization don't + # follow the same rule; we often don't want spaces before those. match = Match(r'^(.*[^ ({>]){', line) + if match: # Try a bit harder to check for brace initialization. This # happens in one of the following forms: @@ -3358,6 +3495,7 @@ def CheckBracesSpacing(filename, clean_lines, linenum, error): # There is a false negative with this approach if people inserted # spurious semicolons, e.g. "if (cond){};", but we will catch the # spurious semicolon with a separate check. + leading_text = match.group(1) (endline, endlinenum, endpos) = CloseExpression( clean_lines, linenum, len(match.group(1))) trailing_text = '' @@ -3366,7 +3504,11 @@ def CheckBracesSpacing(filename, clean_lines, linenum, error): for offset in xrange(endlinenum + 1, min(endlinenum + 3, clean_lines.NumLines() - 1)): trailing_text += clean_lines.elided[offset] - if not Match(r'^[\s}]*[{.;,)<>\]:]', trailing_text): + # We also suppress warnings for `uint64_t{expression}` etc., as the style + # guide recommends brace initialization for integral types to avoid + # overflow/truncation. + if (not Match(r'^[\s}]*[{.;,)<>\]:]', trailing_text) + and not _IsType(clean_lines, nesting_state, leading_text)): error(filename, linenum, 'whitespace/braces', 5, 'Missing space before {') @@ -3410,405 +3552,6 @@ def IsDecltype(clean_lines, linenum, column): return False -def IsTemplateParameterList(clean_lines, linenum, column): - """Check if the token ending on (linenum, column) is the end of template<>. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: the number of the line to check. - column: end column of the token to check. - Returns: - True if this token is end of a template parameter list, False otherwise. - """ - (_, startline, startpos) = ReverseCloseExpression( - clean_lines, linenum, column) - if (startpos > -1 and - Search(r'\btemplate\s*$', clean_lines.elided[startline][0:startpos])): - return True - return False - - -def IsRValueType(typenames, clean_lines, nesting_state, linenum, column): - """Check if the token ending on (linenum, column) is a type. - - Assumes that text to the right of the column is "&&" or a function - name. - - Args: - typenames: set of type names from template-argument-list. - clean_lines: A CleansedLines instance containing the file. - nesting_state: A NestingState instance which maintains information about - the current stack of nested blocks being parsed. - linenum: the number of the line to check. - column: end column of the token to check. - Returns: - True if this token is a type, False if we are not sure. - """ - prefix = clean_lines.elided[linenum][0:column] - - # Get one word to the left. If we failed to do so, this is most - # likely not a type, since it's unlikely that the type name and "&&" - # would be split across multiple lines. - match = Match(r'^(.*)(\b\w+|[>*)&])\s*$', prefix) - if not match: - return False - - # Check text following the token. If it's "&&>" or "&&," or "&&...", it's - # most likely a rvalue reference used inside a template. - suffix = clean_lines.elided[linenum][column:] - if Match(r'&&\s*(?:[>,]|\.\.\.)', suffix): - return True - - # Check for known types and end of templates: - # int&& variable - # vector&& variable - # - # Because this function is called recursively, we also need to - # recognize pointer and reference types: - # int* Function() - # int& Function() - if (match.group(2) in typenames or - match.group(2) in ['char', 'char16_t', 'char32_t', 'wchar_t', 'bool', - 'short', 'int', 'long', 'signed', 'unsigned', - 'float', 'double', 'void', 'auto', '>', '*', '&']): - return True - - # If we see a close parenthesis, look for decltype on the other side. - # decltype would unambiguously identify a type, anything else is - # probably a parenthesized expression and not a type. - if match.group(2) == ')': - return IsDecltype( - clean_lines, linenum, len(match.group(1)) + len(match.group(2)) - 1) - - # Check for casts and cv-qualifiers. - # match.group(1) remainder - # -------------- --------- - # const_cast< type&& - # const type&& - # type const&& - if Search(r'\b(?:const_cast\s*<|static_cast\s*<|dynamic_cast\s*<|' - r'reinterpret_cast\s*<|\w+\s)\s*$', - match.group(1)): - return True - - # Look for a preceding symbol that might help differentiate the context. - # These are the cases that would be ambiguous: - # match.group(1) remainder - # -------------- --------- - # Call ( expression && - # Declaration ( type&& - # sizeof ( type&& - # if ( expression && - # while ( expression && - # for ( type&& - # for( ; expression && - # statement ; type&& - # block { type&& - # constructor { expression && - start = linenum - line = match.group(1) - match_symbol = None - while start >= 0: - # We want to skip over identifiers and commas to get to a symbol. - # Commas are skipped so that we can find the opening parenthesis - # for function parameter lists. - match_symbol = Match(r'^(.*)([^\w\s,])[\w\s,]*$', line) - if match_symbol: - break - start -= 1 - line = clean_lines.elided[start] - - if not match_symbol: - # Probably the first statement in the file is an rvalue reference - return True - - if match_symbol.group(2) == '}': - # Found closing brace, probably an indicate of this: - # block{} type&& - return True - - if match_symbol.group(2) == ';': - # Found semicolon, probably one of these: - # for(; expression && - # statement; type&& - - # Look for the previous 'for(' in the previous lines. - before_text = match_symbol.group(1) - for i in xrange(start - 1, max(start - 6, 0), -1): - before_text = clean_lines.elided[i] + before_text - if Search(r'for\s*\([^{};]*$', before_text): - # This is the condition inside a for-loop - return False - - # Did not find a for-init-statement before this semicolon, so this - # is probably a new statement and not a condition. - return True - - if match_symbol.group(2) == '{': - # Found opening brace, probably one of these: - # block{ type&& = ... ; } - # constructor{ expression && expression } - - # Look for a closing brace or a semicolon. If we see a semicolon - # first, this is probably a rvalue reference. - line = clean_lines.elided[start][0:len(match_symbol.group(1)) + 1] - end = start - depth = 1 - while True: - for ch in line: - if ch == ';': - return True - elif ch == '{': - depth += 1 - elif ch == '}': - depth -= 1 - if depth == 0: - return False - end += 1 - if end >= clean_lines.NumLines(): - break - line = clean_lines.elided[end] - # Incomplete program? - return False - - if match_symbol.group(2) == '(': - # Opening parenthesis. Need to check what's to the left of the - # parenthesis. Look back one extra line for additional context. - before_text = match_symbol.group(1) - if linenum > 1: - before_text = clean_lines.elided[linenum - 1] + before_text - before_text = match_symbol.group(1) - - # Patterns that are likely to be types: - # [](type&& - # for (type&& - # sizeof(type&& - # operator=(type&& - # - if Search(r'(?:\]|\bfor|\bsizeof|\boperator\s*\S+\s*)\s*$', before_text): - return True - - # Patterns that are likely to be expressions: - # if (expression && - # while (expression && - # : initializer(expression && - # , initializer(expression && - # ( FunctionCall(expression && - # + FunctionCall(expression && - # + (expression && - # - # The last '+' represents operators such as '+' and '-'. - if Search(r'(?:\bif|\bwhile|[-+=%^(]*>)?\s*$', - match_symbol.group(1)) - if match_func: - # Check for constructors, which don't have return types. - if Search(r'\b(?:explicit|inline)$', match_func.group(1)): - return True - implicit_constructor = Match(r'\s*(\w+)\((?:const\s+)?(\w+)', prefix) - if (implicit_constructor and - implicit_constructor.group(1) == implicit_constructor.group(2)): - return True - return IsRValueType(typenames, clean_lines, nesting_state, linenum, - len(match_func.group(1))) - - # Nothing before the function name. If this is inside a block scope, - # this is probably a function call. - return not (nesting_state.previous_stack_top and - nesting_state.previous_stack_top.IsBlockInfo()) - - if match_symbol.group(2) == '>': - # Possibly a closing bracket, check that what's on the other side - # looks like the start of a template. - return IsTemplateParameterList( - clean_lines, start, len(match_symbol.group(1))) - - # Some other symbol, usually something like "a=b&&c". This is most - # likely not a type. - return False - - -def 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, typenames): - """Check if RValue reference is allowed on a particular line. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - typenames: set of type names from template-argument-list. - 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): - if not line.endswith('PUSH'): - return False - for j in xrange(linenum, clean_lines.NumLines(), 1): - line = clean_lines.elided[j] - if Match(r'GOOGLE_ALLOW_RVALUE_REFERENCES_(?:PUSH|POP)', line): - return line.endswith('POP') - - # 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<>]+::)*([\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) - - # Reject types not mentioned in template-argument-list - while line: - match = Match(r'^.*?(\w+)\s*&&(.*)$', line) - if not match: - break - if match.group(1) not in typenames: - return False - line = match.group(2) - - # All RValue types that were in template-argument-list should have - # been removed by now. Those were allowed, assuming that they will - # be forwarded. - # - # If there are no remaining RValue types left (i.e. types that were - # not found in template-argument-list), flag those as not allowed. - return line.find('&&') < 0 - - -def GetTemplateArgs(clean_lines, linenum): - """Find list of template arguments associated with this function declaration. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: Line number containing the start of the function declaration, - usually one line after the end of the template-argument-list. - Returns: - Set of type names, or empty set if this does not appear to have - any template parameters. - """ - # Find start of function - func_line = linenum - while func_line > 0: - line = clean_lines.elided[func_line] - if Match(r'^\s*$', line): - return set() - if line.find('(') >= 0: - break - func_line -= 1 - if func_line == 0: - return set() - - # Collapse template-argument-list into a single string - argument_list = '' - match = Match(r'^(\s*template\s*)<', clean_lines.elided[func_line]) - if match: - # template-argument-list on the same line as function name - start_col = len(match.group(1)) - _, end_line, end_col = CloseExpression(clean_lines, func_line, start_col) - if end_col > -1 and end_line == func_line: - start_col += 1 # Skip the opening bracket - argument_list = clean_lines.elided[func_line][start_col:end_col] - - elif func_line > 1: - # template-argument-list one line before function name - match = Match(r'^(.*)>\s*$', clean_lines.elided[func_line - 1]) - if match: - end_col = len(match.group(1)) - _, start_line, start_col = ReverseCloseExpression( - clean_lines, func_line - 1, end_col) - if start_col > -1: - start_col += 1 # Skip the opening bracket - while start_line < func_line - 1: - argument_list += clean_lines.elided[start_line][start_col:] - start_col = 0 - start_line += 1 - argument_list += clean_lines.elided[func_line - 1][start_col:end_col] - - if not argument_list: - return set() - - # Extract type names - typenames = set() - while True: - match = Match(r'^[,\s]*(?:typename|class)(?:\.\.\.)?\s+(\w+)(.*)$', - argument_list) - if not match: - break - typenames.add(match.group(1)) - argument_list = match.group(2) - return typenames - - -def CheckRValueReference(filename, clean_lines, linenum, nesting_state, error): - """Check for rvalue references. - - Args: - filename: The name of the current file. - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - nesting_state: A NestingState instance which maintains information about - the current stack of nested blocks being parsed. - error: The function to call with any errors found. - """ - # Find lines missing spaces around &&. - # TODO(unknown): currently we don't check for rvalue references - # with spaces surrounding the && to avoid false positives with - # boolean expressions. - line = clean_lines.elided[linenum] - match = Match(r'^(.*\S)&&', line) - if not match: - match = Match(r'(.*)&&\S', line) - if (not match) or '(&&)' in line or Search(r'\boperator\s*$', match.group(1)): - return - - # Either poorly formed && or an rvalue reference, check the context - # to get a more accurate error message. Mostly we want to determine - # if what's to the left of "&&" is a type or not. - typenames = GetTemplateArgs(clean_lines, linenum) - and_pos = len(match.group(1)) - if IsRValueType(typenames, clean_lines, nesting_state, linenum, and_pos): - if not IsRValueAllowed(clean_lines, linenum, typenames): - error(filename, linenum, 'build/c++11', 3, - 'RValue references are an unapproved C++ feature.') - else: - error(filename, linenum, 'whitespace/operators', 3, - 'Missing spaces around &&') - - def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error): """Checks for additional blank line issues related to sections. @@ -3906,10 +3649,13 @@ def CheckBraces(filename, clean_lines, linenum, error): # used for brace initializers inside function calls. We don't detect this # perfectly: we just don't complain if the last non-whitespace character on # the previous non-blank line is ',', ';', ':', '(', '{', or '}', or if the - # previous line starts a preprocessor block. + # previous line starts a preprocessor block. We also allow a brace on the + # following line if it is part of an array initialization and would not fit + # within the 80 character limit of the preceding line. prevline = GetPreviousNonBlankLine(clean_lines, linenum)[0] if (not Search(r'[,;:}{(]\s*$', prevline) and - not Match(r'\s*#', prevline)): + not Match(r'\s*#', prevline) and + not (GetLineWidth(prevline) > _line_length - 2 and '[]' in prevline)): error(filename, linenum, 'whitespace/braces', 4, '{ should almost always be at the end of the previous line') @@ -4085,13 +3831,14 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): # In addition to macros, we also don't want to warn on # - Compound literals # - Lambdas - # - alignas specifier with anonymous structs: + # - alignas specifier with anonymous structs + # - decltype closing_brace_pos = match.group(1).rfind(')') opening_parenthesis = ReverseCloseExpression( clean_lines, linenum, closing_brace_pos) if opening_parenthesis[2] > -1: line_prefix = opening_parenthesis[0][0:opening_parenthesis[2]] - macro = Search(r'\b([A-Z_]+)\s*$', line_prefix) + macro = Search(r'\b([A-Z_][A-Z0-9_]*)\s*$', line_prefix) func = Match(r'^(.*\])\s*$', line_prefix) if ((macro and macro.group(1) not in ( @@ -4100,6 +3847,7 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): 'LOCKS_EXCLUDED', 'INTERFACE_DEF')) or (func and not Search(r'\boperator\s*\[\s*\]', func.group(1))) or Search(r'\b(?:struct|union)\s+alignas\s*$', line_prefix) or + Search(r'\bdecltype$', line_prefix) or Search(r'\s+=\s*$', line_prefix)): match = None if (match and @@ -4159,7 +3907,7 @@ def CheckEmptyBlockBody(filename, clean_lines, linenum, error): line = clean_lines.elided[linenum] matched = Match(r'\s*(for|while|if)\s*\(', line) if matched: - # Find the end of the conditional expression + # Find the end of the conditional expression. (end_line, end_linenum, end_pos) = CloseExpression( clean_lines, linenum, line.find('(')) @@ -4174,6 +3922,75 @@ def CheckEmptyBlockBody(filename, clean_lines, linenum, error): error(filename, end_linenum, 'whitespace/empty_loop_body', 5, 'Empty loop bodies should use {} or continue') + # Check for if statements that have completely empty bodies (no comments) + # and no else clauses. + if end_pos >= 0 and matched.group(1) == 'if': + # Find the position of the opening { for the if statement. + # Return without logging an error if it has no brackets. + opening_linenum = end_linenum + opening_line_fragment = end_line[end_pos:] + # Loop until EOF or find anything that's not whitespace or opening {. + while not Search(r'^\s*\{', opening_line_fragment): + if Search(r'^(?!\s*$)', opening_line_fragment): + # Conditional has no brackets. + return + opening_linenum += 1 + if opening_linenum == len(clean_lines.elided): + # Couldn't find conditional's opening { or any code before EOF. + return + opening_line_fragment = clean_lines.elided[opening_linenum] + # Set opening_line (opening_line_fragment may not be entire opening line). + opening_line = clean_lines.elided[opening_linenum] + + # Find the position of the closing }. + opening_pos = opening_line_fragment.find('{') + if opening_linenum == end_linenum: + # We need to make opening_pos relative to the start of the entire line. + opening_pos += end_pos + (closing_line, closing_linenum, closing_pos) = CloseExpression( + clean_lines, opening_linenum, opening_pos) + if closing_pos < 0: + return + + # Now construct the body of the conditional. This consists of the portion + # of the opening line after the {, all lines until the closing line, + # and the portion of the closing line before the }. + if (clean_lines.raw_lines[opening_linenum] != + CleanseComments(clean_lines.raw_lines[opening_linenum])): + # Opening line ends with a comment, so conditional isn't empty. + return + if closing_linenum > opening_linenum: + # Opening line after the {. Ignore comments here since we checked above. + body = list(opening_line[opening_pos+1:]) + # All lines until closing line, excluding closing line, with comments. + body.extend(clean_lines.raw_lines[opening_linenum+1:closing_linenum]) + # Closing line before the }. Won't (and can't) have comments. + body.append(clean_lines.elided[closing_linenum][:closing_pos-1]) + body = '\n'.join(body) + else: + # If statement has brackets and fits on a single line. + body = opening_line[opening_pos+1:closing_pos-1] + + # Check if the body is empty + if not _EMPTY_CONDITIONAL_BODY_PATTERN.search(body): + return + # The body is empty. Now make sure there's not an else clause. + current_linenum = closing_linenum + current_line_fragment = closing_line[closing_pos:] + # Loop until EOF or find anything that's not whitespace or else clause. + while Search(r'^\s*$|^(?=\s*else)', current_line_fragment): + if Search(r'^(?=\s*else)', current_line_fragment): + # Found an else clause, so don't log an error. + return + current_linenum += 1 + if current_linenum == len(clean_lines.elided): + break + current_line_fragment = clean_lines.elided[current_linenum] + + # The body is empty and there's no else clause until EOF or other code. + error(filename, end_linenum, 'whitespace/empty_if_body', 4, + ('If statement had no body and no else clause')) + def FindCheckMacro(line): """Find a replaceable CHECK-like macro. @@ -4393,6 +4210,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # raw strings, raw_lines = clean_lines.lines_without_raw_strings line = raw_lines[linenum] + prev = raw_lines[linenum - 1] if linenum > 0 else '' if line.find('\t') != -1: error(filename, linenum, 'whitespace/tab', 1, @@ -4416,19 +4234,24 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, cleansed_line = clean_lines.elided[linenum] while initial_spaces < len(line) and line[initial_spaces] == ' ': initial_spaces += 1 - if line and line[-1].isspace(): - error(filename, linenum, 'whitespace/end_of_line', 4, - 'Line ends in whitespace. Consider deleting these extra spaces.') # There are certain situations we allow one space, notably for # section labels, and also lines containing multi-line raw strings. - elif ((initial_spaces == 1 or initial_spaces == 3) and - not Match(scope_or_label_pattern, cleansed_line) and - not (clean_lines.raw_lines[linenum] != line and - Match(r'^\s*""', line))): + # We also don't check for lines that look like continuation lines + # (of lines ending in double quotes, commas, equals, or angle brackets) + # because the rules for how to indent those are non-trivial. + if (not Search(r'[",=><] *$', prev) and + (initial_spaces == 1 or initial_spaces == 3) and + not Match(scope_or_label_pattern, cleansed_line) and + not (clean_lines.raw_lines[linenum] != line and + Match(r'^\s*""', line))): error(filename, linenum, 'whitespace/indent', 3, 'Weird number of spaces at line-start. ' 'Are you using a 2-space indent?') + if line and line[-1].isspace(): + error(filename, linenum, 'whitespace/end_of_line', 4, + 'Line ends in whitespace. Consider deleting these extra spaces.') + # Check if the line is a header guard. is_header_guard = False if file_extension == 'h': @@ -4447,14 +4270,10 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # developers fault. if (not line.startswith('#include') and not is_header_guard and not Match(r'^\s*//.*http(s?)://\S*$', line) and + not Match(r'^\s*//\s*[^\s]*$', line) and not Match(r'^// \$Id:.*#[0-9]+ \$$', line)): line_width = GetLineWidth(line) - extended_length = int((_line_length * 1.25)) - if line_width > extended_length: - error(filename, linenum, 'whitespace/line_length', 4, - 'Lines should very rarely be longer than %i characters' % - extended_length) - elif line_width > _line_length: + if line_width > _line_length: error(filename, linenum, 'whitespace/line_length', 2, 'Lines should be <= %i characters long' % _line_length) @@ -4479,9 +4298,8 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, CheckOperatorSpacing(filename, clean_lines, linenum, error) CheckParenthesisSpacing(filename, clean_lines, linenum, error) CheckCommaSpacing(filename, clean_lines, linenum, error) - CheckBracesSpacing(filename, clean_lines, linenum, error) + CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error) CheckSpacingForFunctionCall(filename, clean_lines, linenum, error) - CheckRValueReference(filename, clean_lines, linenum, nesting_state, error) CheckCheck(filename, clean_lines, linenum, error) CheckAltTokens(filename, clean_lines, linenum, error) classinfo = nesting_state.InnermostClass() @@ -4525,23 +4343,6 @@ def _DropCommonSuffixes(filename): return os.path.splitext(filename)[0] -def _IsTestFilename(filename): - """Determines if the given filename has a suffix that identifies it as a test. - - Args: - filename: The input filename. - - Returns: - True if 'filename' looks like a test, False otherwise. - """ - if (filename.endswith('_test.cc') or - filename.endswith('_unittest.cc') or - filename.endswith('_regtest.cc')): - return True - else: - return False - - def _ClassifyInclude(fileinfo, include, is_system): """Figures out what kind of header 'include' is. @@ -4756,6 +4557,9 @@ def _GetTextInside(text, start_pattern): _RE_PATTERN_CONST_REF_PARAM = ( r'(?:.*\s*\bconst\s*&\s*' + _RE_PATTERN_IDENT + r'|const\s+' + _RE_PATTERN_TYPE + r'\s*&\s*' + _RE_PATTERN_IDENT + r')') +# Stream types. +_RE_PATTERN_REF_STREAM_PARAM = ( + r'(?:.*stream\s*&\s*' + _RE_PATTERN_IDENT + r')') def CheckLanguage(filename, clean_lines, linenum, file_extension, @@ -4794,7 +4598,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, # Make Windows paths like Unix. fullname = os.path.abspath(filename).replace('\\', '/') - + # Perform other checks now that we are sure that this is not an include line CheckCasts(filename, clean_lines, linenum, error) CheckGlobalStatic(filename, clean_lines, linenum, error) @@ -4933,9 +4737,13 @@ def CheckGlobalStatic(filename, clean_lines, linenum, error): # 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. + # globals with constructors are initialized before the first access, and + # also because globals can be destroyed when some threads are still running. + # TODO(unknown): Generalize this to also find static unique_ptr instances. + # TODO(unknown): File bugs for clang-tidy to find these. match = Match( - r'((?:|static +)(?:|const +))string +([a-zA-Z0-9_:]+)\b(.*)', + r'((?:|static +)(?:|const +))(?::*std::)?string( +const)? +' + r'([a-zA-Z0-9_:]+)\b(.*)', line) # Remove false positives: @@ -4955,15 +4763,20 @@ def CheckGlobalStatic(filename, clean_lines, linenum, error): # matching identifiers. # string Class::operator*() if (match and - not Search(r'\bstring\b(\s+const)?\s*\*\s*(const\s+)?\w', line) and + not Search(r'\bstring\b(\s+const)?\s*[\*\&]\s*(const\s+)?\w', line) and not Search(r'\boperator\W', line) and - not Match(r'\s*(<.*>)?(::[a-zA-Z0-9_]+)*\s*\(([^"]|$)', match.group(3))): - error(filename, linenum, 'runtime/string', 4, - 'For a static/global string constant, use a C style string instead: ' - '"%schar %s[]".' % - (match.group(1), match.group(2))) + not Match(r'\s*(<.*>)?(::[a-zA-Z0-9_]+)*\s*\(([^"]|$)', match.group(4))): + if Search(r'\bconst\b', line): + error(filename, linenum, 'runtime/string', 4, + 'For a static/global string constant, use a C style string ' + 'instead: "%schar%s %s[]".' % + (match.group(1), match.group(2) or '', match.group(3))) + else: + error(filename, linenum, 'runtime/string', 4, + 'Static/global string variables are not permitted.') - if Search(r'\b([A-Za-z0-9_]*_)\(\1\)', line): + if (Search(r'\b([A-Za-z0-9_]*_)\(\1\)', line) or + Search(r'\b([A-Za-z0-9_]*_)\(CHECK_NOTNULL\(\1\)\)', line)): error(filename, linenum, 'runtime/init', 4, 'You seem to be initializing a member variable with itself.') @@ -5208,7 +5021,8 @@ def CheckForNonConstReference(filename, clean_lines, linenum, decls = ReplaceAll(r'{[^}]*}', ' ', line) # exclude function body for parameter in re.findall(_RE_PATTERN_REF_PARAM, decls): - if not Match(_RE_PATTERN_CONST_REF_PARAM, parameter): + if (not Match(_RE_PATTERN_CONST_REF_PARAM, parameter) and + not Match(_RE_PATTERN_REF_STREAM_PARAM, parameter)): error(filename, linenum, 'runtime/references', 2, 'Is this a non-const reference? ' 'If so, make const or use a pointer: ' + @@ -5231,7 +5045,7 @@ def CheckCasts(filename, clean_lines, linenum, error): # Parameterless conversion functions, such as bool(), are allowed as they are # probably a member operator declaration or default constructor. match = Search( - r'(\bnew\s+|\S<\s*(?:const\s+)?)?\b' + r'(\bnew\s+(?:const\s+)?|\S<\s*(?:const\s+)?)?\b' r'(int|float|double|bool|char|int32|uint32|int64|uint64)' r'(\([^)].*)', line) expecting_function = ExpectingFunctionArgs(clean_lines, linenum) @@ -5372,63 +5186,12 @@ def CheckCStyleCast(filename, clean_lines, linenum, cast_type, pattern, error): if context.endswith(' operator++') or context.endswith(' operator--'): return False - # A single unnamed argument for a function tends to look like old - # style cast. If we see those, don't issue warnings for deprecated - # casts, instead issue warnings for unnamed arguments where - # appropriate. - # - # These are things that we want warnings for, since the style guide - # explicitly require all parameters to be named: - # Function(int); - # Function(int) { - # ConstMember(int) const; - # ConstMember(int) const { - # ExceptionMember(int) throw (...); - # ExceptionMember(int) throw (...) { - # PureVirtual(int) = 0; - # [](int) -> bool { - # - # These are functions of some sort, where the compiler would be fine - # if they had named parameters, but people often omit those - # identifiers to reduce clutter: - # (FunctionPointer)(int); - # (FunctionPointer)(int) = value; - # Function((function_pointer_arg)(int)) - # Function((function_pointer_arg)(int), int param) - # ; - # <(FunctionPointerTemplateArgument)(int)>; + # A single unnamed argument for a function tends to look like old style cast. + # If we see those, don't issue warnings for deprecated casts. remainder = line[match.end(0):] if Match(r'^\s*(?:;|const\b|throw\b|final\b|override\b|[=>{),]|->)', remainder): - # Looks like an unnamed parameter. - - # Don't warn on any kind of template arguments. - if Match(r'^\s*>', remainder): - return False - - # Don't warn on assignments to function pointers, but keep warnings for - # unnamed parameters to pure virtual functions. Note that this pattern - # will also pass on assignments of "0" to function pointers, but the - # preferred values for those would be "nullptr" or "NULL". - matched_zero = Match(r'^\s=\s*(\S+)\s*;', remainder) - if matched_zero and matched_zero.group(1) != '0': - return False - - # Don't warn on function pointer declarations. For this we need - # to check what came before the "(type)" string. - if Match(r'.*\)\s*$', line[0:match.start(0)]): - return False - - # 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 - - # Passed all filters, issue warning here. - error(filename, linenum, 'readability/function', 3, - 'All parameters should be named in a function') - return True + return False # At this point, all that should be left is actual casts. error(filename, linenum, 'readability/casting', 4, @@ -5498,18 +5261,26 @@ def ExpectingFunctionArgs(clean_lines, linenum): ('', ('slist',)), ) -_RE_PATTERN_STRING = re.compile(r'\bstring\b') +_HEADERS_MAYBE_TEMPLATES = ( + ('', ('copy', 'max', 'min', 'min_element', 'sort', + 'transform', + )), + ('', ('swap',)), + ) -_re_pattern_algorithm_header = [] -for _template in ('copy', 'max', 'min', 'min_element', 'sort', 'swap', - 'transform'): - # Match max(..., ...), max(..., ...), but not foo->max, foo.max or - # type::max(). - _re_pattern_algorithm_header.append( - (re.compile(r'[^>.]\b' + _template + r'(<.*?>)?\([^\)]'), - _template, - '')) +_RE_PATTERN_STRING = re.compile(r'\bstring\b') +_re_pattern_headers_maybe_templates = [] +for _header, _templates in _HEADERS_MAYBE_TEMPLATES: + for _template in _templates: + # Match max(..., ...), max(..., ...), but not foo->max, foo.max or + # type::max(). + _re_pattern_headers_maybe_templates.append( + (re.compile(r'[^>.]\b' + _template + r'(<.*?>)?\([^\)]'), + _template, + _header)) + +# Other scripts may reach in and modify this pattern. _re_pattern_templates = [] for _header, _templates in _HEADERS_CONTAINING_TEMPLATES: for _template in _templates: @@ -5549,13 +5320,13 @@ def FilesBelongToSameModule(filename_cc, filename_h): string: the additional prefix needed to open the header file. """ - if not filename_cc.endswith('.cc'): + fileinfo = FileInfo(filename_cc) + if not fileinfo.IsSource(): return (False, '') - filename_cc = filename_cc[:-len('.cc')] - if filename_cc.endswith('_unittest'): - filename_cc = filename_cc[:-len('_unittest')] - elif filename_cc.endswith('_test'): - filename_cc = filename_cc[:-len('_test')] + filename_cc = filename_cc[:-len(fileinfo.Extension())] + matched_test_suffix = Search(_TEST_FILE_SUFFIX, fileinfo.BaseName()) + if matched_test_suffix: + filename_cc = filename_cc[:-len(matched_test_suffix.group(1))] filename_cc = filename_cc.replace('/public/', '/') filename_cc = filename_cc.replace('/internal/', '/') @@ -5636,7 +5407,7 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, if prefix.endswith('std::') or not prefix.endswith('::'): required[''] = (linenum, 'string') - for pattern, template, header in _re_pattern_algorithm_header: + for pattern, template, header in _re_pattern_headers_maybe_templates: if pattern.search(line): required[header] = (linenum, template) @@ -5719,31 +5490,6 @@ def CheckMakePairUsesDeduction(filename, clean_lines, linenum, error): ' OR use pair directly OR if appropriate, construct a pair directly') -def CheckDefaultLambdaCaptures(filename, clean_lines, linenum, error): - """Check that default lambda captures are not used. - - Args: - filename: The name of the current file. - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - error: The function to call with any errors found. - """ - line = clean_lines.elided[linenum] - - # A lambda introducer specifies a default capture if it starts with "[=" - # or if it starts with "[&" _not_ followed by an identifier. - match = Match(r'^(.*)\[\s*(?:=|&[^\w])', line) - if match: - # Found a potential error, check what comes after the lambda-introducer. - # If it's not open parenthesis (for lambda-declarator) or open brace - # (for compound-statement), it's not a lambda. - line, _, pos = CloseExpression(clean_lines, linenum, len(match.group(1))) - if pos >= 0 and Match(r'^\s*[{(]', line[pos:]): - error(filename, linenum, 'build/c++11', - 4, # 4 = high confidence - 'Default lambda captures are an unapproved C++ feature.') - - def CheckRedundantVirtual(filename, clean_lines, linenum, error): """Check if line contains a redundant "virtual" function-specifier. @@ -5942,7 +5688,6 @@ def ProcessLine(filename, file_extension, clean_lines, line, CheckPosixThreading(filename, clean_lines, line, error) CheckInvalidIncrement(filename, clean_lines, line, error) CheckMakePairUsesDeduction(filename, clean_lines, line, error) - 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: @@ -5959,8 +5704,14 @@ def FlagCxx11Features(filename, clean_lines, linenum, error): """ line = clean_lines.elided[linenum] - # Flag unapproved C++11 headers. include = Match(r'\s*#\s*include\s+[<"]([^<"]+)[">]', line) + + # Flag unapproved C++ TR1 headers. + if include and include.group(1).startswith('tr1/'): + error(filename, linenum, 'build/c++tr1', 5, + ('C++ TR1 headers such as <%s> are unapproved.') % include.group(1)) + + # Flag unapproved C++11 headers. if include and include.group(1) in ('cfenv', 'condition_variable', 'fenv.h', @@ -5994,6 +5745,25 @@ def FlagCxx11Features(filename, clean_lines, linenum, error): 'they may let you use it.') % top_name) +def FlagCxx14Features(filename, clean_lines, linenum, error): + """Flag those C++14 features that we restrict. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] + + include = Match(r'\s*#\s*include\s+[<"]([^<"]+)[">]', line) + + # Flag unapproved C++14 headers. + if include and include.group(1) in ('scoped_allocator', 'shared_mutex'): + error(filename, linenum, 'build/c++14', 5, + ('<%s> is an unapproved C++14 header.') % include.group(1)) + + def ProcessFileData(filename, file_extension, lines, error, extra_check_functions=[]): """Performs lint checks and reports any errors to the given error function. @@ -6019,7 +5789,7 @@ def ProcessFileData(filename, file_extension, lines, error, ResetNolintSuppressions() CheckForCopyright(filename, lines, error) - + ProcessGlobalSuppresions(lines) RemoveMultiLineComments(filename, lines, error) clean_lines = CleansedLines(lines) @@ -6034,9 +5804,9 @@ def ProcessFileData(filename, file_extension, lines, error, nesting_state.CheckCompletedBlocks(filename, error) CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error) - + # Check that the .cc file has included its header if it exists. - if file_extension == 'cc': + if _IsSourceExtension(file_extension): CheckHeaderFileIncluded(filename, include_state, error) # We check here rather than inside ProcessLine so that we see raw diff --git a/cpplint/cpplint_unittest.py b/cpplint/cpplint_unittest.py index 8d9eae6aa..fd059fb01 100755 --- a/cpplint/cpplint_unittest.py +++ b/cpplint/cpplint_unittest.py @@ -290,6 +290,14 @@ def testNameSpaceIndentationNoError(self): results = self.GetNamespaceResults(lines) self.assertEquals(results, '') + def testWhitespaceBeforeNamespace(self): + lines = [' namespace Test {', + ' void foo() { }', + ' } // namespace Test'] + + results = self.GetNamespaceResults(lines) + self.assertEquals(results, '') + def testFalsePositivesNoError(self): lines = ['namespace Test {', 'struct OuterClass {', @@ -362,13 +370,23 @@ def testLineLengthCheck(self): '// Hello', '') self.TestLint( - '// ' + 'x' * 80, + '// x' + ' x' * 40, 'Lines should be <= 80 characters long' ' [whitespace/line_length] [2]') self.TestLint( - '// ' + 'x' * 100, - 'Lines should very rarely be longer than 100 characters' - ' [whitespace/line_length] [4]') + '// x' + ' x' * 50, + 'Lines should be <= 80 characters long' + ' [whitespace/line_length] [2]') + self.TestLint( + '// //some/path/to/f' + ('i' * 100) + 'le', + '') + self.TestLint( + '// //some/path/to/f' + ('i' * 100) + 'le', + '') + self.TestLint( + '// //some/path/to/f' + ('i' * 50) + 'le and some comments', + 'Lines should be <= 80 characters long' + ' [whitespace/line_length] [2]') self.TestLint( '// http://g' + ('o' * 100) + 'gle.com/', '') @@ -445,6 +463,64 @@ def testErrorSuppression(self): ''], error_collector) self.assertEquals('', error_collector.Results()) + # LINT_C_FILE silences cast warnings for entire file. + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('test.h', 'h', + ['// Copyright 2014 Your Company.', + '// NOLINT(build/header_guard)', + 'int64 a = (uint64) 65;', + '// LINT_C_FILE', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) + # Vim modes silence cast warnings for entire file. + for modeline in ['vi:filetype=c', + 'vi:sw=8 filetype=c', + 'vi:sw=8 filetype=c ts=8', + 'vi: filetype=c', + 'vi: sw=8 filetype=c', + 'vi: sw=8 filetype=c ts=8', + 'vim:filetype=c', + 'vim:sw=8 filetype=c', + 'vim:sw=8 filetype=c ts=8', + 'vim: filetype=c', + 'vim: sw=8 filetype=c', + 'vim: sw=8 filetype=c ts=8', + 'vim: set filetype=c:', + 'vim: set sw=8 filetype=c:', + 'vim: set sw=8 filetype=c ts=8:', + 'vim: set filetype=c :', + 'vim: set sw=8 filetype=c :', + 'vim: set sw=8 filetype=c ts=8 :', + 'vim: se filetype=c:', + 'vim: se sw=8 filetype=c:', + 'vim: se sw=8 filetype=c ts=8:', + 'vim: se filetype=c :', + 'vim: se sw=8 filetype=c :', + 'vim: se sw=8 filetype=c ts=8 :']: + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('test.h', 'h', + ['// Copyright 2014 Your Company.', + '// NOLINT(build/header_guard)', + 'int64 a = (uint64) 65;', + '/* Prevent warnings about the modeline', + modeline, + '*/', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) + # LINT_KERNEL_FILE silences whitespace/tab warnings for entire file. + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('test.h', 'h', + ['// Copyright 2014 Your Company.', + '// NOLINT(build/header_guard)', + 'struct test {', + '\tint member;', + '};', + '// LINT_KERNEL_FILE', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) # Test Variable Declarations. def testVariableDeclarations(self): @@ -509,14 +585,55 @@ def testCStyleCast(self): self.TestLint('[](int/*unused*/) -> bool {', '') self.TestLint('[](int /*unused*/) -> bool {', '') self.TestLint('auto f = [](MyStruct* /*unused*/)->int {', '') - self.TestLint( - '[](int) -> bool {', - 'All parameters should be named in a function' - ' [readability/function] [3]') - self.TestLint( - 'auto f = [](MyStruct*)->int {', - 'All parameters should be named in a function' - ' [readability/function] [3]') + self.TestLint('[](int) -> bool {', '') + self.TestLint('auto f = [](MyStruct*)->int {', '') + + # Cast with brace initializers + self.TestLint('int64_t{4096} * 1000 * 1000', '') + self.TestLint('size_t{4096} * 1000 * 1000', '') + self.TestLint('uint_fast16_t{4096} * 1000 * 1000', '') + + # Brace initializer with templated type + self.TestMultiLineLint( + """ + template + void Function(int arg1, + int arg2) { + variable &= ~Type1{0} - 1; + }""", + '') + self.TestMultiLineLint( + """ + template + class Class { + void Function() { + variable &= ~Type{0} - 1; + } + };""", + '') + self.TestMultiLineLint( + """ + template + class Class { + void Function() { + variable &= ~Type{0} - 1; + } + };""", + '') + self.TestMultiLineLint( + """ + namespace { + template + class Class { + void Function() { + if (block) { + variable &= ~Type{0} - 1; + } + } + }; + }""", + '') # Test taking address of casts (runtime/casting) def testRuntimeCasting(self): @@ -559,6 +676,10 @@ def testRuntimeSelfinit(self): 'Foo::Foo(Bar r, Bel l) : r_(r_), l_(l_) { }', 'You seem to be initializing a member variable with itself.' ' [runtime/init] [4]') + self.TestLint( + 'Foo::Foo(Bar r, Bel l) : r_(CHECK_NOTNULL(r_)) { }', + 'You seem to be initializing a member variable with itself.' + ' [runtime/init] [4]') self.TestLint( 'Foo::Foo(Bar r, Bel l) : r_(r), l_(l) { }', '') @@ -568,14 +689,12 @@ def testRuntimeSelfinit(self): # Test for unnamed arguments in a method. def testCheckForUnnamedParams(self): - message = ('All parameters should be named in a function' - ' [readability/function] [3]') - self.TestLint('virtual void Func(int*) const;', message) - self.TestLint('virtual void Func(int*);', message) - self.TestLint('void Method(char*) {', message) - self.TestLint('void Method(char*);', message) - self.TestLint('static void operator delete[](void*) throw();', message) - self.TestLint('int Method(int);', message) + self.TestLint('virtual void Func(int*) const;', '') + self.TestLint('virtual void Func(int*);', '') + self.TestLint('void Method(char*) {', '') + self.TestLint('void Method(char*);', '') + self.TestLint('static void operator delete[](void*) throw();', '') + self.TestLint('int Method(int);', '') self.TestLint('virtual void Func(int* p);', '') self.TestLint('void operator delete(void* x) throw();', '') @@ -627,6 +746,7 @@ def testDeprecatedCast(self): 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('new const int(42);', '') # "new" on const-qualified type self.TestLint('using a = bool(int arg);', '') # C++11 alias-declaration self.TestLint('x = bit_cast(y);', '') # array of array self.TestLint('void F(const char(&src)[N]);', '') # array of references @@ -838,7 +958,7 @@ def testIncludeWhatYouUse(self): """#include "base/foobar.h" bool foobar = swap(0,1); """, - 'Add #include for swap [build/include_what_you_use] [4]') + 'Add #include for swap [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( """#include "base/foobar.h" bool foobar = transform(a.begin(), a.end(), b.start(), Foo); @@ -1199,16 +1319,13 @@ class Foo { };""", 'Zero-parameter constructors should not be marked explicit.' ' [runtime/explicit] [5]') - # Warn explicit multi-argument constructors at lowest severity + # No warning for multi-parameter constructors 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 { @@ -1863,18 +1980,6 @@ def testCheckCheck(self): 'EXPECT_TRUE(+42 >= x);', 'Consider using EXPECT_GE instead of EXPECT_TRUE(a >= b)' ' [readability/check] [2]') - self.TestLint( - '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);', - 'Consider using EXPECT_LE_M instead of EXPECT_TRUE_M(a <= b)' - ' [readability/check] [2]') - self.TestLint( - '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);', @@ -1896,10 +2001,6 @@ def testCheckCheck(self): 'ASSERT_FALSE(x <= 42);', 'Consider using ASSERT_GT instead of ASSERT_FALSE(a <= b)' ' [readability/check] [2]') - self.TestLint( - '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);', ['Missing spaces around <' @@ -2033,6 +2134,10 @@ def testNonConstReference(self): self.TestLint('stream& operator>>(stream& s, Foo& f);', '') self.TestLint('stream& operator<<(stream& s, Foo& f);', '') self.TestLint('void swap(Bar& a, Bar& b);', '') + self.TestLint('ostream& LogFunc(ostream& s);', '') + self.TestLint('ostringstream& LogFunc(ostringstream& s);', '') + self.TestLint('istream& LogFunc(istream& s);', '') + self.TestLint('istringstream& LogFunc(istringstream& s);', '') # Returning a non-const reference from a function is OK. self.TestLint('int& g();', '') # Passing a const reference to a struct (using the struct keyword) is OK. @@ -2282,6 +2387,9 @@ def testSpacingForFncall(self): self.TestLint('((a+b))', '') self.TestLint('foo (foo)', 'Extra space before ( in function call' ' [whitespace/parens] [4]') + # asm volatile () may have a space, as it isn't a function call. + self.TestLint('asm volatile ("")', '') + self.TestLint('__asm__ __volatile__ ("")', '') self.TestLint('} catch (const Foo& ex) {', '') self.TestLint('case (42):', '') self.TestLint('typedef foo (*foo)(foo)', '') @@ -2319,9 +2427,20 @@ def testSpacingBeforeBraces(self): self.TestLint('for {', '') self.TestLint('EXPECT_DEBUG_DEATH({', '') self.TestLint('std::is_convertible{}', '') + self.TestLint('blah{32}', 'Missing space before {' + ' [whitespace/braces] [5]') + self.TestLint('int8_t{3}', '') + self.TestLint('int16_t{3}', '') + self.TestLint('int32_t{3}', '') + self.TestLint('uint64_t{12345}', '') + self.TestLint('constexpr int64_t kBatchGapMicros =' + ' int64_t{7} * 24 * 3600 * 1000000; // 1 wk.', '') + self.TestLint('MoveOnly(int i1, int i2) : ip1{new int{i1}}, ' + 'ip2{new int{i2}} {}', + '') def testSemiColonAfterBraces(self): - self.TestLint('if (cond) {};', + self.TestLint('if (cond) { func(); };', 'You don\'t need a ; after a } [readability/braces] [4]') self.TestLint('void Func() {};', 'You don\'t need a ; after a } [readability/braces] [4]') @@ -2337,8 +2456,11 @@ def testSemiColonAfterBraces(self): self.TestLint('class X : public Y {};', '') self.TestLint('class X : public MACRO() {};', '') + self.TestLint('class X : public decltype(expr) {};', '') self.TestLint('DEFINE_FACADE(PCQueue::Watcher, PCQueue) {};', '') self.TestLint('VCLASS(XfaTest, XfaContextTest) {};', '') + self.TestLint('class STUBBY_CLASS(H, E) {};', '') + self.TestLint('class STUBBY2_CLASS(H, E) {};', '') self.TestLint('TEST(TestCase, TestName) {};', 'You don\'t need a ; after a } [readability/braces] [4]') self.TestLint('TEST_F(TestCase, TestName) {};', @@ -2365,19 +2487,6 @@ def testLambda(self): '};\n', '') - for lambda_with_default_capture in ('void f() { [=]{}; }', - 'void f() { [=](int i) {}; }', - 'void f() { [=, &x]{}; }', - 'void f() { [&]{}; }', - 'void f() { [ & ]{}; }', - 'void f() { [&, y]{}; }'): - self.TestLint(lambda_with_default_capture, - 'Default lambda captures are an unapproved C++ feature. ' - '[build/c++11] [4]') - - # "[&...]" isn't necessarily a default capture, though "[=...]" always is. - self.TestLint('void f() { [&variable]{}; }', '') - # Avoid false positives with operator[] self.TestLint('table_to_children[&*table].push_back(dependent);', '') @@ -2412,13 +2521,28 @@ def testBraceInitializerList(self): ' }\n' '};\n', '') self.TestMultiLineLint('if (true) {\n' - ' if (false){}\n' + ' if (false){ func(); }\n' '}\n', 'Missing space before { [whitespace/braces] [5]') self.TestMultiLineLint('MyClass::MyClass()\n' ' : initializer_{\n' ' Func()} {\n' '}\n', '') + self.TestLint('const pair kCL' + + ('o' * 41) + 'gStr[] = {\n', + 'Lines should be <= 80 characters long' + ' [whitespace/line_length] [2]') + self.TestMultiLineLint('const pair kCL' + + ('o' * 40) + 'ngStr[] =\n' + ' {\n' + ' {"gooooo", "oooogle"},\n' + '};\n', '') + self.TestMultiLineLint('const pair kCL' + + ('o' * 39) + 'ngStr[] =\n' + ' {\n' + ' {"gooooo", "oooogle"},\n' + '};\n', '{ should almost always be at the end of ' + 'the previous line [whitespace/braces] [4]') def testSpacingAroundElse(self): self.TestLint('}else {', 'Missing space before else' @@ -2455,6 +2579,7 @@ def testSpacingForBinaryOps(self): 'Missing spaces around << [whitespace/operators] [3]') self.TestLint('a<>b', 'Missing spaces around >> [whitespace/operators] [3]') @@ -2481,141 +2606,6 @@ def testSpacingForBinaryOps(self): self.TestLint('using Vector3::operator==;', '') self.TestLint('using Vector3::operator!=;', '') - def testRvalueReference(self): - space_error = 'Missing spaces around && [whitespace/operators] [3]' - rvalue_error = ('RValue references are an unapproved C++ feature.' - ' [build/c++11] [3]') - - # Places where lack of space are allowed - self.TestLint('DEFINE_BINARY_OPERATOR(&&)', '') - self.TestLint('bool operator&&(A b) {}', '') - self.TestLint('bool operator&&(A b) {', '') - self.TestLint('bool operator&&(A b);', '') - - # Assignment expressions - self.TestLint('a = b && c;', '') - self.TestLint('a = b&& c;', space_error) - self.TestLint('a = b &&c;', space_error) - self.TestLint('a = (b&& c);', space_error) - self.TestLint('a = (b &&c);', space_error) - self.TestLint('a&& b = c;', rvalue_error) - self.TestLint('a&& c = d;', rvalue_error) - self.TestLint('auto&& a = b;', rvalue_error) - self.TestLint('const a&& b = c;', rvalue_error) - self.TestLint('struct a&& b = c;', rvalue_error) - self.TestLint('decltype(a)&& b = c;', rvalue_error) - - # Cast expressions - self.TestLint('a = const_cast(c);', rvalue_error) - self.TestLint('a = const_cast(c);', rvalue_error) - self.TestLint('a = static_cast(c);', rvalue_error) - self.TestLint('a = static_cast(c);', rvalue_error) - self.TestLint('a = dynamic_cast(c);', rvalue_error) - self.TestLint('a = dynamic_cast(c);', rvalue_error) - self.TestLint('a = reinterpret_cast(c);', rvalue_error) - self.TestLint('a = reinterpret_cast(c);', rvalue_error) - self.TestLint('a = cast < b&& c;', space_error) - - # Function parameters - for indent in ['', ' ']: - for head in ['void Func', 'vector Func', 'vector\nFunc', - 'inline void Func', - 'Constructor', 'Constructor::Constructor', - 'operator=', 'operator =', 'operator = ']: - for body in [' {}', ' {', ';']: - self.TestMultiLineLint(indent + head + '(A&& b)' + body, rvalue_error) - self.TestMultiLineLint(indent + head + '(A &&b)' + body, rvalue_error) - self.TestMultiLineLint(indent + head + '(A&&... b)' + body, - rvalue_error) - self.TestMultiLineLint(indent + head + '(A&& c)' + body, - rvalue_error) - self.TestMultiLineLint(indent + head + '(A &&c)' + body, - rvalue_error) - - # Function templates - self.TestLint('std::conditional::type', rvalue_error) - self.TestLint('std::conditional::type', rvalue_error) - - # Template functions - self.TestLint('template R&& F()', rvalue_error) - self.TestLint('template R&& F() {', rvalue_error) - self.TestMultiLineLint('template \nR&& F()', rvalue_error) - self.TestMultiLineLint('template \nR&& F() {', rvalue_error) - self.TestLint('template void F(T a, R&& b)', rvalue_error) - self.TestLint('template void F(T a, R &&b)', rvalue_error) - self.TestLint('template void F(T a, R&& b) {', rvalue_error) - - # For loops - self.TestLint('for (a&& b;;)', rvalue_error) - self.TestLint('for (a&& b;;) {', rvalue_error) - self.TestLint('for (; a&& b;)', space_error) - self.TestLint('for (; a&& b;) {', space_error) - - # Constructors - self.TestLint('A(a&& b)', rvalue_error) - self.TestLint('explicit A(a&& b)', rvalue_error) - self.TestLint('A(a b) : c(d&& e)', space_error) - self.TestLint('A(a b) : c(), d(e&& f)', space_error) - - 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 2014 Your Company.', - 'GOOGLE_ALLOW_RVALUE_REFERENCES_PUSH', - 'void F(A&& b);', - 'GOOGLE_ALLOW_RVALUE_REFERENCES_POP', - ''], - 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)', - ' inline X(X&& param) = default; // NOLINT(runtime/explicit)', - '', - ' X& operator=(X&& param) = delete;', - ' X& operator=(X&& param) = default;', - '};', - 'A::A(A&&) = default;', - 'Outer::Inner::Inner(Inner&&) = default;', - ''], - error_collector) - self.assertEquals(error_collector.Results(), '') - - # Assume templated function parameters are forwarded, and are allowed - error_collector = ErrorCollector(self.assert_) - cpplint.ProcessFileData( - 'foo.cc', 'cc', - ['// Copyright 2014 Your Company.', - 'template ', - 'void Function1(Allowed1&& a);', - '', - 'template ', - 'void Function2(Allowed2&& a, Allowed3 &&b) {', - '}', - '', - 'template ', - 'void Function3(Ignored1 *a, Allowed4&& b) {', - '}', - '', - 'template ', - 'void Function4(Allowed5&&... a) {', - '}', - '', - 'template ', - 'void Function5(', - ' Allowed6 &&...a) {', - '}', - ''], - error_collector) - self.assertEquals(error_collector.Results(), '') - def testSpacingBeforeLastSemicolon(self): self.TestLint('call_function() ;', 'Extra space before last semicolon. If this should be an ' @@ -2649,6 +2639,11 @@ def testEmptyBlockBody(self): self.TestLint('for (;;)', '') self.TestLint('for (;;) continue;', '') self.TestLint('for (;;) func();', '') + self.TestLint('if (test) {}', + 'If statement had no body and no else clause' + ' [whitespace/empty_if_body] [4]') + self.TestLint('if (test) func();', '') + self.TestLint('if (test) {} else {}', '') self.TestMultiLineLint("""while (true && false);""", 'Empty loop bodies should use {} or continue' @@ -2665,6 +2660,39 @@ def testEmptyBlockBody(self): while (false);""", 'Empty loop bodies should use {} or continue' ' [whitespace/empty_loop_body] [5]') + self.TestMultiLineLint("""if (test) { + }""", + 'If statement had no body and no else clause' + ' [whitespace/empty_if_body] [4]') + self.TestMultiLineLint("""if (test, + func({})) { + }""", + 'If statement had no body and no else clause' + ' [whitespace/empty_if_body] [4]') + self.TestMultiLineLint("""if (test) + func();""", '') + self.TestLint('if (test) { hello; }', '') + self.TestLint('if (test({})) { hello; }', '') + self.TestMultiLineLint("""if (test) { + func(); + }""", '') + self.TestMultiLineLint("""if (test) { + // multiline + // comment + }""", '') + self.TestMultiLineLint("""if (test) { // comment + }""", '') + self.TestMultiLineLint("""if (test) { + } else { + }""", '') + self.TestMultiLineLint("""if (func(p1, + p2, + p3)) { + func(); + }""", '') + self.TestMultiLineLint("""if (func({}, p1)) { + func(); + }""", '') def testSpacingForRangeBasedFor(self): # Basic correctly formatted case: @@ -2693,19 +2721,49 @@ def testSpacingForRangeBasedFor(self): # Static or global STL strings. def testStaticOrGlobalSTLStrings(self): + # A template for the error message for a const global/static string. error_msg = ('For a static/global string constant, use a C style ' 'string instead: "%s[]". [runtime/string] [4]') + # The error message for a non-const global/static string variable. + nonconst_error_msg = ('Static/global string variables are not permitted.' + ' [runtime/string] [4]') + self.TestLint('string foo;', - error_msg % 'char foo') + nonconst_error_msg) self.TestLint('string kFoo = "hello"; // English', - error_msg % 'char kFoo') + nonconst_error_msg) self.TestLint('static string foo;', - error_msg % 'static char foo') + nonconst_error_msg) self.TestLint('static const string foo;', error_msg % 'static const char foo') + self.TestLint('static const std::string foo;', + error_msg % 'static const char foo') self.TestLint('string Foo::bar;', - error_msg % 'char Foo::bar') + nonconst_error_msg) + + self.TestLint('std::string foo;', + nonconst_error_msg) + self.TestLint('std::string kFoo = "hello"; // English', + nonconst_error_msg) + self.TestLint('static std::string foo;', + nonconst_error_msg) + self.TestLint('static const std::string foo;', + error_msg % 'static const char foo') + self.TestLint('std::string Foo::bar;', + nonconst_error_msg) + + self.TestLint('::std::string foo;', + nonconst_error_msg) + self.TestLint('::std::string kFoo = "hello"; // English', + nonconst_error_msg) + self.TestLint('static ::std::string foo;', + nonconst_error_msg) + self.TestLint('static const ::std::string foo;', + error_msg % 'static const char foo') + self.TestLint('::std::string Foo::bar;', + nonconst_error_msg) + self.TestLint('string* pointer', '') self.TestLint('string *pointer', '') self.TestLint('string* pointer = Func();', '') @@ -2725,12 +2783,14 @@ def testStaticOrGlobalSTLStrings(self): self.TestLint('string Foo::bar() {}', '') self.TestLint('string Foo::operator*() {}', '') # Rare case. - self.TestLint('string foo("foobar");', error_msg % 'char foo') + self.TestLint('string foo("foobar");', nonconst_error_msg) # Should not catch local or member variables. self.TestLint(' string foo', '') # Should not catch functions. self.TestLint('string EmptyString() { return ""; }', '') self.TestLint('string EmptyString () { return ""; }', '') + self.TestLint('string const& FileInfo::Pathname() const;', '') + self.TestLint('string const &FileInfo::Pathname() const;', '') self.TestLint('string VeryLongNameFunctionSometimesEndsWith(\n' ' VeryLongNameType very_long_name_variable) {}', '') self.TestLint('template<>\n' @@ -2761,21 +2821,24 @@ def testStaticOrGlobalSTLStrings(self): 'NestedClass::MemberFunction3();', 'string TemplateClass::', 'NestedClass::MemberFunction4();', - 'string Class', + 'const string Class', '::static_member_variable1;', - 'string Class::', + 'const string Class::', 'static_member_variable2;', - 'string Class', + 'const string Class', '::static_member_variable3 = "initial value";', - 'string Class::', + 'const string Class::', 'static_member_variable4 = "initial value";', + 'string Class::', + 'static_member_variable5;', ''], 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']) + [error_msg % 'const char Class::static_member_variable1', + error_msg % 'const char Class::static_member_variable2', + error_msg % 'const char Class::static_member_variable3', + error_msg % 'const char Class::static_member_variable4', + nonconst_error_msg]) def testNoSpacesInFunctionCalls(self): self.TestLint('TellStory(1, 3);', @@ -2825,6 +2888,9 @@ def testToDoComments(self): self.TestLint('// TODO(ljenkins): Fix this', '') self.TestLint('#if 1 // TEST_URLTODOCID_WHICH_HAS_THAT_WORD_IN_IT_H_', '') self.TestLint('// See also similar TODO above', '') + self.TestLint(r'EXPECT_EQ("\\", ' + r'NormalizePath("/./../foo///bar/..//x/../..", ""));', + '') def testTwoSpacesBetweenCodeAndComments(self): self.TestLint('} // namespace foo', @@ -3360,10 +3426,9 @@ class foo { '') self.TestMultiLineLint( TrimExtraIndent(''' - static const char kNotRawString[] = "(" - ")";'''), - 'Weird number of spaces at line-start. ' - 'Are you using a 2-space indent? [whitespace/indent] [3]') + KV>'''), + '') self.TestMultiLineLint( ' static const char kSingleLineRawString[] = R"(...)";', 'Weird number of spaces at line-start. ' @@ -3723,18 +3788,18 @@ def testLineLength(self): try: cpplint._line_length = 80 self.TestLint( - '// %s' % ('H' * 77), + '// H %s' % ('H' * 75), '') self.TestLint( - '// %s' % ('H' * 78), + '// H %s' % ('H' * 76), 'Lines should be <= 80 characters long' ' [whitespace/line_length] [2]') cpplint._line_length = 120 self.TestLint( - '// %s' % ('H' * 117), + '// H %s' % ('H' * 115), '') self.TestLint( - '// %s' % ('H' * 118), + '// H %s' % ('H' * 116), 'Lines should be <= 120 characters long' ' [whitespace/line_length] [2]') finally: @@ -3873,7 +3938,8 @@ def GetBuildHeaderGuardPreprocessorSymbol(self, file_path): cpplint.ProcessFileData(file_path, 'h', [], error_collector) for error in error_collector.ResultList(): matched = re.search( - 'No #ifndef header guard found, suggested CPP variable is: ([A-Z_]+)', + 'No #ifndef header guard found, suggested CPP variable is: ' + '([A-Z0-9_]+)', error) if matched is not None: return matched.group(1) @@ -4176,8 +4242,8 @@ def testBuildStorageClass(self): storage_classes = ['extern', 'register', 'static', 'typedef'] build_storage_class_error_message = ( - 'Storage class (static, extern, typedef, etc) should be first.' - ' [build/storage_class] [5]') + 'Storage-class specifier (static, extern, typedef, etc) should be ' + 'at the beginning of the declaration. [build/storage_class] [5]') # Some explicit cases. Legal in C++, deprecated in C99. self.TestLint('const int static foo = 5;', @@ -4309,6 +4375,9 @@ def TestCxx11Feature(self, code, expected_error): self.assertEquals(expected_error, collector.Results()) def testBlockedHeaders(self): + self.TestCxx11Feature('#include ', + 'C++ TR1 headers such as are ' + 'unapproved. [build/c++tr1] [5]') self.TestCxx11Feature('#include ', ' is an unapproved C++11 header.' ' [build/c++11] [5]') @@ -4353,6 +4422,25 @@ def testExplicitMakePair(self): ' [build/explicit_make_pair] [4]') self.TestLint('my_make_pair', '') +class Cxx14Test(CpplintTestBase): + + def TestCxx14Feature(self, code, expected_error): + lines = code.split('\n') + collector = ErrorCollector(self.assert_) + cpplint.RemoveMultiLineComments('foo.h', lines, collector) + clean_lines = cpplint.CleansedLines(lines) + cpplint.FlagCxx14Features('foo.cc', clean_lines, 0, collector) + self.assertEquals(expected_error, collector.Results()) + + def testBlockedHeaders(self): + self.TestCxx14Feature('#include ', + ' is an unapproved C++14 header.' + ' [build/c++14] [5]') + self.TestCxx14Feature('#include ', + ' is an unapproved C++14 header.' + ' [build/c++14] [5]') + + class CleansedLinesTest(unittest.TestCase): def testInit(self): @@ -4970,6 +5058,20 @@ def testFunctionLengthNotDeterminable(self): 'Lint failed to find start of function body.' ' [readability/fn_size] [5]') + def testFunctionLengthCheckWithNamespace(self): + old_verbosity = cpplint._SetVerboseLevel(1) + self.TestFunctionLengthsCheck( + ('namespace {\n' + 'void CodeCoverageCL35256059() {\n' + + (' X++;\n' * 3000) + + '}\n' + '} // namespace\n'), + ('Small and focused functions are preferred: ' + 'CodeCoverageCL35256059() has 3000 non-comment lines ' + '(error triggered by exceeding 20 lines).' + ' [readability/fn_size] [5]')) + cpplint._SetVerboseLevel(old_verbosity) + def TrimExtraIndent(text_block): """Trim a uniform amount of whitespace off of each line in a string.