diff --git a/cpplint/cpplint.py b/cpplint/cpplint.py index 66b1d9b..ccc25d4 100755 --- a/cpplint/cpplint.py +++ b/cpplint/cpplint.py @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/env python # # Copyright (c) 2009 Google Inc. All rights reserved. # @@ -175,71 +175,77 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] # If you add a new error message with a new category, add it to the list # here! cpplint_unittest.py should tell you if you forget to do this. _ERROR_CATEGORIES = [ - 'build/class', - 'build/c++11', - 'build/deprecated', - 'build/endif_comment', - 'build/explicit_make_pair', - 'build/forward_decl', - 'build/header_guard', - 'build/include', - 'build/include_alpha', - 'build/include_order', - 'build/include_what_you_use', - 'build/namespaces', - 'build/printf_format', - 'build/storage_class', - 'legal/copyright', - 'readability/alt_tokens', - 'readability/braces', - 'readability/casting', - 'readability/check', - 'readability/constructors', - 'readability/fn_size', - 'readability/function', - 'readability/inheritance', - 'readability/multiline_comment', - 'readability/multiline_string', - 'readability/namespace', - 'readability/nolint', - 'readability/nul', - 'readability/streams', - 'readability/todo', - 'readability/utf8', - 'runtime/arrays', - 'runtime/casting', - 'runtime/explicit', - 'runtime/int', - 'runtime/init', - 'runtime/invalid_increment', - 'runtime/member_string_references', - 'runtime/memset', - 'runtime/indentation_namespace', - 'runtime/operator', - 'runtime/printf', - 'runtime/printf_format', - 'runtime/references', - 'runtime/string', - 'runtime/threadsafe_fn', - 'runtime/vlog', - 'whitespace/blank_line', - 'whitespace/braces', - 'whitespace/comma', - 'whitespace/comments', - 'whitespace/empty_conditional_body', - 'whitespace/empty_loop_body', - 'whitespace/end_of_line', - 'whitespace/ending_newline', - 'whitespace/forcolon', - 'whitespace/indent', - 'whitespace/line_length', - 'whitespace/newline', - 'whitespace/operators', - 'whitespace/parens', - 'whitespace/semicolon', - 'whitespace/tab', - 'whitespace/todo' - ] + 'build/class', + 'build/c++11', + 'build/deprecated', + 'build/endif_comment', + 'build/explicit_make_pair', + 'build/forward_decl', + 'build/header_guard', + 'build/include', + 'build/include_alpha', + 'build/include_order', + 'build/include_what_you_use', + 'build/namespaces', + 'build/printf_format', + 'build/storage_class', + 'legal/copyright', + 'readability/alt_tokens', + 'readability/braces', + 'readability/casting', + 'readability/check', + 'readability/constructors', + 'readability/fn_size', + 'readability/function', + 'readability/inheritance', + 'readability/multiline_comment', + 'readability/multiline_string', + 'readability/namespace', + 'readability/nolint', + 'readability/nul', + 'readability/strings', + 'readability/todo', + 'readability/utf8', + 'runtime/arrays', + 'runtime/casting', + 'runtime/explicit', + 'runtime/int', + 'runtime/init', + 'runtime/invalid_increment', + 'runtime/member_string_references', + 'runtime/memset', + 'runtime/indentation_namespace', + 'runtime/operator', + 'runtime/printf', + 'runtime/printf_format', + 'runtime/references', + 'runtime/string', + 'runtime/threadsafe_fn', + 'runtime/vlog', + 'whitespace/blank_line', + 'whitespace/braces', + 'whitespace/comma', + 'whitespace/comments', + 'whitespace/empty_conditional_body', + 'whitespace/empty_loop_body', + 'whitespace/end_of_line', + 'whitespace/ending_newline', + 'whitespace/forcolon', + 'whitespace/indent', + 'whitespace/line_length', + 'whitespace/newline', + 'whitespace/operators', + 'whitespace/parens', + 'whitespace/semicolon', + 'whitespace/tab', + 'whitespace/todo', + ] + +# These error categories are no longer enforced by cpplint, but for backwards- +# compatibility they may still appear in NOLINT comments. +_LEGACY_ERROR_CATEGORIES = [ + 'readability/streams', + ] # The default state of the category filter. This is overridden by the --filter= # flag. By default all errors are on, so only add here categories that should be @@ -522,7 +528,7 @@ def ParseNolintSuppressions(filename, raw_line, linenum, error): category = category[1:-1] if category in _ERROR_CATEGORIES: _error_suppressions.setdefault(category, set()).add(suppressed_line) - else: + elif category not in _LEGACY_ERROR_CATEGORIES: error(filename, linenum, 'readability/nolint', 5, 'Unknown NOLINT error category: %s' % category) @@ -690,7 +696,7 @@ class _IncludeState(object): # If previous line was a blank line, assume that the headers are # intentionally sorted the way they are. if (self._last_header > header_path and - not Match(r'^\s*$', clean_lines.elided[linenum - 1])): + Match(r'^\s*#\s*include\b', clean_lines.elided[linenum - 1])): return False return True @@ -1246,7 +1252,7 @@ def RemoveMultiLineCommentsFromRange(lines, begin, end): # Having // dummy comments makes the lines non-empty, so we will not get # unnecessary blank line warnings later in the code. for i in range(begin, end): - lines[i] = '// dummy' + lines[i] = '/**/' def RemoveMultiLineComments(filename, lines, error): @@ -1282,12 +1288,14 @@ def CleanseComments(line): class CleansedLines(object): - """Holds 3 copies of all lines with different preprocessing applied to them. + """Holds 4 copies of all lines with different preprocessing applied to them. - 1) elided member contains lines without strings and comments, - 2) lines member contains lines without comments, and + 1) elided member contains lines without strings and comments. + 2) lines member contains lines without comments. 3) raw_lines member contains all the lines without processing. - All these three members are of , and of the same length. + 4) lines_without_raw_strings member is same as raw_lines, but with C++11 raw + strings removed. + All these members are of , and of the same length. """ def __init__(self, lines): @@ -1656,15 +1664,17 @@ def GetHeaderGuardCPPVariable(filename): # flymake. filename = re.sub(r'_flymake\.h$', '.h', 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: file_path_from_root = re.sub('^' + _root + os.sep, '', file_path_from_root) - return re.sub(r'[-./\s]', '_', file_path_from_root).upper() + '_' + return re.sub(r'[^a-zA-Z0-9]', '_', file_path_from_root).upper() + '_' -def CheckForHeaderGuard(filename, lines, error): +def CheckForHeaderGuard(filename, clean_lines, error): """Checks that the file contains a header guard. Logs an error if no #ifndef header guard is present. For other @@ -1672,7 +1682,7 @@ def CheckForHeaderGuard(filename, lines, error): Args: filename: The name of the C++ header file. - lines: An array of strings, each representing a line of the file. + clean_lines: A CleansedLines instance containing the file. error: The function to call with any errors found. """ @@ -1682,18 +1692,19 @@ def CheckForHeaderGuard(filename, lines, error): # 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: + raw_lines = clean_lines.lines_without_raw_strings + for i in raw_lines: if Search(r'//\s*NOLINT\(build/header_guard\)', i): return cppvar = GetHeaderGuardCPPVariable(filename) - ifndef = None + ifndef = '' ifndef_linenum = 0 - define = None - endif = None + define = '' + endif = '' endif_linenum = 0 - for linenum, line in enumerate(lines): + for linenum, line in enumerate(raw_lines): linesplit = line.split() if len(linesplit) >= 2: # find the first occurrence of #ifndef and #define, save arg @@ -1708,18 +1719,12 @@ def CheckForHeaderGuard(filename, lines, error): endif = line endif_linenum = linenum - if not ifndef: + if not ifndef or not define or ifndef != define: error(filename, 0, 'build/header_guard', 5, 'No #ifndef header guard found, suggested CPP variable is: %s' % cppvar) return - if not define: - error(filename, 0, 'build/header_guard', 5, - 'No #define header guard found, suggested CPP variable is: %s' % - cppvar) - return - # The guard should be PATH_FILE_H_, but we also allow PATH_FILE_H__ # for backward compatibility. if ifndef != cppvar: @@ -1727,26 +1732,69 @@ def CheckForHeaderGuard(filename, lines, error): if ifndef != cppvar + '_': error_level = 5 - ParseNolintSuppressions(filename, lines[ifndef_linenum], ifndef_linenum, + ParseNolintSuppressions(filename, raw_lines[ifndef_linenum], ifndef_linenum, error) error(filename, ifndef_linenum, 'build/header_guard', error_level, '#ifndef header guard has wrong style, please use: %s' % cppvar) - if define != ifndef: - error(filename, 0, 'build/header_guard', 5, - '#ifndef and #define don\'t match, suggested CPP variable is: %s' % - cppvar) + # Check for "//" comments on endif line. + ParseNolintSuppressions(filename, raw_lines[endif_linenum], endif_linenum, + error) + match = Match(r'#endif\s*//\s*' + cppvar + r'(_)?\b', endif) + if match: + if match.group(1) == '_': + # Issue low severity warning for deprecated double trailing underscore + error(filename, endif_linenum, 'build/header_guard', 0, + '#endif line should be "#endif // %s"' % cppvar) return - if endif != ('#endif // %s' % cppvar): - error_level = 0 - if endif != ('#endif // %s' % (cppvar + '_')): - error_level = 5 + # Didn't find the corresponding "//" comment. If this file does not + # contain any "//" comments at all, it could be that the compiler + # only wants "/**/" comments, look for those instead. + no_single_line_comments = True + for i in xrange(1, len(raw_lines) - 1): + line = raw_lines[i] + if Match(r'^(?:(?:\'(?:\.|[^\'])*\')|(?:"(?:\.|[^"])*")|[^\'"])*//', line): + no_single_line_comments = False + break - ParseNolintSuppressions(filename, lines[endif_linenum], endif_linenum, - error) - error(filename, endif_linenum, 'build/header_guard', error_level, - '#endif line should be "#endif // %s"' % cppvar) + if no_single_line_comments: + match = Match(r'#endif\s*/\*\s*' + cppvar + r'(_)?\s*\*/', endif) + if match: + if match.group(1) == '_': + # Low severity warning for double trailing underscore + error(filename, endif_linenum, 'build/header_guard', 0, + '#endif line should be "#endif /* %s */"' % cppvar) + return + + # Didn't find anything + error(filename, endif_linenum, 'build/header_guard', 5, + '#endif line should be "#endif // %s"' % cppvar) + + +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'): + return + + fileinfo = FileInfo(filename) + headerfile = filename[0:len(filename) - 2] + 'h' + if not os.path.exists(headerfile): + return + headername = FileInfo(headerfile).RepositoryName() + first_include = 0 + for section_list in include_state.include_list: + for f in section_list: + if headername in f[0] or f[0] in headername: + return + if not first_include: + first_include = f[1] + + error(filename, first_include, 'build/include', 5, + '%s should include its header file %s' % (fileinfo.RepositoryName(), + headername)) def CheckForBadCharacters(filename, lines, error): @@ -2042,6 +2090,23 @@ class _ClassInfo(_BlockInfo): self.is_derived = True def CheckEnd(self, filename, clean_lines, linenum, error): + # If there is a DISALLOW macro, it should appear near the end of + # the class. + seen_last_thing_in_class = False + for i in xrange(linenum - 1, self.starting_linenum, -1): + match = Search( + r'\b(DISALLOW_COPY_AND_ASSIGN|DISALLOW_IMPLICIT_CONSTRUCTORS)\(' + + self.name + r'\)', + clean_lines.elided[i]) + if match: + if seen_last_thing_in_class: + error(filename, i, 'readability/constructors', 3, + match.group(1) + ' should be the last thing in the class') + break + + if not Match(r'^\s*$', clean_lines.elided[i]): + seen_last_thing_in_class = True + # Check that closing brace is aligned with beginning of the class. # Only do this if the closing brace is indented by only whitespaces. # This means we will not check single-line class definitions. @@ -2722,7 +2787,8 @@ def CheckSpacingForFunctionCall(filename, clean_lines, linenum, error): 'Extra space after (') if (Search(r'\w\s+\(', fncall) and not Search(r'#\s*define|typedef|using\s+\w+\s*=', fncall) and - not Search(r'\w\s+\((\w+::)*\*\w+\)\(', fncall)): + not Search(r'\w\s+\((\w+::)*\*\w+\)\(', fncall) and + not Search(r'\bcase\s+\(', fncall)): # TODO(unknown): Space after an operator function seem to be a common # error, silence those for now by restricting them to highest verbosity. if Search(r'\boperator_*\b', line): @@ -2892,11 +2958,14 @@ def CheckComment(line, filename, linenum, next_line_start, error): 'TODO(my_username) should be followed by a space') # If the comment contains an alphanumeric character, there - # should be a space somewhere between it and the //. - if Match(r'//[^ ]*\w', comment): + # should be a space somewhere between it and the // unless + # it's a /// or //! Doxygen comment. + if (Match(r'//[^ ]*\w', comment) and + not Match(r'(///|//\!)(\s+|$)', comment)): error(filename, linenum, 'whitespace/comments', 4, 'Should have a space between // and comment') + def CheckAccess(filename, clean_lines, linenum, nesting_state, error): """Checks for improper use of DISALLOW* macros. @@ -3083,7 +3152,12 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # Otherwise not. Note we only check for non-spaces on *both* sides; # sometimes people put non-spaces on one side when aligning ='s among # many lines (not that this is behavior that I approve of...) - if Search(r'[\w.]=[\w.]', line) and not Search(r'\b(if|while) ', line): + if ((Search(r'[\w.]=', line) or + Search(r'=[\w.]', line)) + and not Search(r'\b(if|while|for) ', line) + # Operators taken from [lex.operators] in C++11 standard. + and not Search(r'(>=|<=|==|!=|&=|\^=|\|=|\+=|\*=|\/=|\%=)', line) + and not Search(r'operator=', line)): error(filename, linenum, 'whitespace/operators', 4, 'Missing spaces around =') @@ -3135,9 +3209,8 @@ 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) - if (match and match.group(1) != '(' and - not (match.group(1).isdigit() and match.group(2).isdigit()) and + match = Search(r'(operator|[^\s(<])(?:L|UL|ULL|l|ul|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, 'Missing spaces around <<') @@ -3255,7 +3328,7 @@ def CheckBracesSpacing(filename, clean_lines, linenum, error): # 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. - match = Match(r'^(.*[^ ({]){', line) + match = Match(r'^(.*[^ ({>]){', line) if match: # Try a bit harder to check for brace initialization. This # happens in one of the following forms: @@ -3355,13 +3428,14 @@ def IsTemplateParameterList(clean_lines, linenum, column): return False -def IsRValueType(clean_lines, nesting_state, linenum, column): +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. @@ -3385,7 +3459,7 @@ def IsRValueType(clean_lines, nesting_state, linenum, column): if Match(r'&&\s*(?:[>,]|\.\.\.)', suffix): return True - # Check for simple type and end of templates: + # Check for known types and end of templates: # int&& variable # vector&& variable # @@ -3393,9 +3467,10 @@ def IsRValueType(clean_lines, nesting_state, linenum, column): # recognize pointer and reference types: # int* Function() # int& Function() - if match.group(2) in ['char', 'char16_t', 'char32_t', 'wchar_t', 'bool', - 'short', 'int', 'long', 'signed', 'unsigned', - 'float', 'double', 'void', 'auto', '>', '*', '&']: + 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. @@ -3528,7 +3603,7 @@ def IsRValueType(clean_lines, nesting_state, linenum, column): # Something else. Check that tokens to the left look like # return_type function_name - match_func = Match(r'^(.*)\s+\w(?:\w|::)*(?:<[^<>]*>)?\s*$', + match_func = Match(r'^(.*\S.*)\s+\w(?:\w|::)*(?:<[^<>]*>)?\s*$', match_symbol.group(1)) if match_func: # Check for constructors, which don't have return types. @@ -3538,7 +3613,7 @@ def IsRValueType(clean_lines, nesting_state, linenum, column): if (implicit_constructor and implicit_constructor.group(1) == implicit_constructor.group(2)): return True - return IsRValueType(clean_lines, nesting_state, linenum, + 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, @@ -3576,12 +3651,13 @@ def IsDeletedOrDefault(clean_lines, linenum): return Match(r'\s*=\s*(?:delete|default)\b', close_line[close_paren:]) -def IsRValueAllowed(clean_lines, linenum): +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. """ @@ -3602,7 +3678,7 @@ def IsRValueAllowed(clean_lines, linenum): return IsDeletedOrDefault(clean_lines, linenum) # Allow constructors - match = Match(r'\s*([\w<>]+)\s*::\s*([\w<>]+)\s*\(', line) + 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): @@ -3615,7 +3691,86 @@ def IsRValueAllowed(clean_lines, linenum): if Match(r'^\s*$', previous_line) or Search(r'[{}:;]\s*$', previous_line): return IsDeletedOrDefault(clean_lines, linenum) - return False + # 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): @@ -3643,9 +3798,10 @@ def CheckRValueReference(filename, clean_lines, linenum, nesting_state, error): # 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(clean_lines, nesting_state, linenum, and_pos): - if not IsRValueAllowed(clean_lines, linenum): + 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: @@ -3926,8 +4082,10 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): # semicolons, while the downside for getting the blacklist wrong # would result in compile errors. # - # In addition to macros, we also don't want to warn on compound - # literals and lambdas. + # In addition to macros, we also don't want to warn on + # - Compound literals + # - Lambdas + # - alignas specifier with anonymous structs: closing_brace_pos = match.group(1).rfind(')') opening_parenthesis = ReverseCloseExpression( clean_lines, linenum, closing_brace_pos) @@ -3941,6 +4099,7 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): 'EXCLUSIVE_LOCKS_REQUIRED', 'SHARED_LOCKS_REQUIRED', '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'\s+=\s*$', line_prefix)): match = None if (match and @@ -4484,6 +4643,10 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): error(filename, linenum, 'build/include', 4, '"%s" already included at %s:%s' % (include, filename, duplicate_line)) + elif (include.endswith('.cc') and + os.path.dirname(fileinfo.RepositoryName()) != os.path.dirname(include)): + error(filename, linenum, 'build/include', 4, + 'Do not include .cc files from other packages') elif not _THIRD_PARTY_HEADERS_PATTERN.match(include): include_state.include_list[-1].append((include, linenum)) @@ -4511,20 +4674,6 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): 'Include "%s" not in alphabetical order' % include) include_state.SetLastHeader(canonical_include) - # Look for any of the stream classes that are part of standard C++. - match = _RE_PATTERN_INCLUDE.match(line) - if match: - include = match.group(2) - if Match(r'(f|ind|io|i|o|parse|pf|stdio|str|)?stream$', include): - # Many unit tests use cout, so we exempt them. - if not _IsTestFilename(filename): - # Suggest a different header for ostream - if include == 'ostream': - error(filename, linenum, 'readability/streams', 3, - 'For logging, include "base/logging.h" instead of .') - else: - error(filename, linenum, 'readability/streams', 3, - 'Streams are highly discouraged.') def _GetTextInside(text, start_pattern): @@ -4755,25 +4904,6 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, 'Do not use variable-length arrays. Use an appropriately named ' "('k' followed by CamelCase) compile-time constant for the size.") - # 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_(COPY_AND_ASSIGN|IMPLICIT_CONSTRUCTORS))' - r'\(.*\);$'), - line) - if match and linenum + 1 < clean_lines.NumLines(): - next_line = clean_lines.elided[linenum + 1] - # We allow some, but not all, declarations of variables to be present - # in the statement that defines the class. The [\w\*,\s]* fragment of - # the regular expression below allows users to declare instances of - # the class or pointers to instances, but not less common types such - # as function pointers or arrays. It's a tradeoff between allowing - # reasonable code and avoiding trying to parse more C++ using regexps. - if not Search(r'^\s*}[\w\*,\s]*;', next_line): - error(filename, linenum, 'readability/constructors', 3, - match.group(1) + ' should be the last thing in the class') - # Check for use of unnamed namespaces in header files. Registration # macros are typically OK, so we allow use of "namespace {" on lines # that end with backslashes. @@ -4889,6 +5019,22 @@ def IsDerivedFunction(clean_lines, linenum): return False +def IsOutOfLineMethodDefinition(clean_lines, linenum): + """Check if current line contains an out-of-line method definition. + + Args: + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + Returns: + True if current line contains an out-of-line method definition. + """ + # Scan back a few lines for start of current function + for i in xrange(linenum, max(-1, linenum - 10), -1): + if Match(r'^([^()]*\w+)\(', clean_lines.elided[i]): + return Match(r'^[^()]*\w+::\w+\(', clean_lines.elided[i]) is not None + return False + + def IsInitializerList(clean_lines, linenum): """Check if current line is inside constructor initializer list. @@ -4957,6 +5103,11 @@ def CheckForNonConstReference(filename, clean_lines, linenum, if IsDerivedFunction(clean_lines, linenum): return + # Don't warn on out-of-line method definitions, as we would warn on the + # in-line declaration, if it isn't marked with 'override'. + if IsOutOfLineMethodDefinition(clean_lines, linenum): + return + # Long type names may be broken across multiple lines, usually in one # of these forms: # LongType @@ -5152,9 +5303,9 @@ def CheckCasts(filename, clean_lines, linenum, error): # This is not a cast: # reference_type&(int* function_param); match = Search( - r'(?:[^\w]&\(([^)]+)\)[\w(])|' + r'(?:[^\w]&\(([^)*][^)]*)\)[\w(])|' r'(?:[^\w]&(static|dynamic|down|reinterpret)_cast\b)', line) - if match and match.group(1) != '*': + if match: # Try a better error message when the & is bound to something # dereferenced by the casted pointer, as opposed to the casted # pointer itself. @@ -5235,6 +5386,7 @@ def CheckCStyleCast(filename, clean_lines, linenum, cast_type, pattern, error): # 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 @@ -5246,7 +5398,7 @@ def CheckCStyleCast(filename, clean_lines, linenum, cast_type, pattern, error): # ; # <(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. @@ -5335,6 +5487,7 @@ _HEADERS_CONTAINING_TEMPLATES = ( ('', ('set', 'multiset',)), ('', ('stack',)), ('', ('char_traits', 'basic_string',)), + ('', ('tuple',)), ('', ('pair',)), ('', ('vector',)), @@ -5602,9 +5755,21 @@ def CheckRedundantVirtual(filename, clean_lines, linenum, error): """ # Look for "virtual" on current line. line = clean_lines.elided[linenum] - virtual = Match(r'^(.*\bvirtual\b)', line) + virtual = Match(r'^(.*)(\bvirtual\b)(.*)$', line) if not virtual: return + # Ignore "virtual" keywords that are near access-specifiers. These + # are only used in class base-specifier and do not apply to member + # functions. + if (Search(r'\b(public|protected|private)\s+$', virtual.group(1)) or + Match(r'^\s+(public|protected|private)\b', virtual.group(3))): + return + + # Ignore the "virtual" keyword from virtual base classes. Usually + # there is a column on the same line in these cases (virtual base + # classes are rare in google3 because multiple inheritance is rare). + if Match(r'^.*[^:]:[^:].*$', line): 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 @@ -5612,7 +5777,7 @@ def CheckRedundantVirtual(filename, clean_lines, linenum, error): # that this is rare. end_col = -1 end_line = -1 - start_col = len(virtual.group(1)) + start_col = len(virtual.group(2)) 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) @@ -5652,9 +5817,21 @@ def CheckRedundantOverrideOrFinal(filename, clean_lines, linenum, error): 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 + # Look for closing parenthesis nearby. We need one to confirm where + # the declarator ends and where the virt-specifier starts to avoid + # false positives. line = clean_lines.elided[linenum] - if Search(r'\boverride\b', line) and Search(r'\bfinal\b', line): + declarator_end = line.rfind(')') + if declarator_end >= 0: + fragment = line[declarator_end:] + else: + if linenum > 1 and clean_lines.elided[linenum - 1].rfind(')') >= 0: + fragment = line + else: + return + + # Check that at most one of "override" or "final" is present, not both + if Search(r'\boverride\b', fragment) and Search(r'\bfinal\b', fragment): error(filename, linenum, 'readability/inheritance', 4, ('"override" is redundant since function is ' 'already declared as "final"')) @@ -5809,9 +5986,6 @@ def FlagCxx11Features(filename, clean_lines, linenum, error): # type_traits 'alignment_of', 'aligned_union', - - # utility - 'forward', ): if Search(r'\bstd::%s\b' % top_name, line): error(filename, linenum, 'build/c++11', 5, @@ -5846,11 +6020,12 @@ def ProcessFileData(filename, file_extension, lines, error, CheckForCopyright(filename, lines, error) - if file_extension == 'h': - CheckForHeaderGuard(filename, lines, error) - RemoveMultiLineComments(filename, lines, error) clean_lines = CleansedLines(lines) + + if file_extension == 'h': + CheckForHeaderGuard(filename, clean_lines, error) + for line in xrange(clean_lines.NumLines()): ProcessLine(filename, file_extension, clean_lines, line, include_state, function_state, nesting_state, error, @@ -5859,6 +6034,10 @@ 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': + CheckHeaderFileIncluded(filename, include_state, error) # We check here rather than inside ProcessLine so that we see raw # lines rather than "cleaned" lines. diff --git a/cpplint/cpplint_unittest.py b/cpplint/cpplint_unittest.py index c380696..aaa3b24 100755 --- a/cpplint/cpplint_unittest.py +++ b/cpplint/cpplint_unittest.py @@ -250,6 +250,58 @@ class CpplintTestBase(unittest.TestCase): class CpplintTest(CpplintTestBase): + def GetNamespaceResults(self, lines): + error_collector = ErrorCollector(self.assert_) + cpplint.RemoveMultiLineComments('foo.h', lines, error_collector) + lines = cpplint.CleansedLines(lines) + nesting_state = cpplint.NestingState() + for i in xrange(lines.NumLines()): + nesting_state.Update('foo.h', lines, i, error_collector) + cpplint.CheckForNamespaceIndentation('foo.h', nesting_state, + lines, i, error_collector) + + return error_collector.Results() + + def testForwardDeclarationNameSpaceIndentation(self): + lines = ['namespace Test {', + ' class ForwardDeclaration;', + '} // namespace Test'] + + results = self.GetNamespaceResults(lines) + self.assertEquals(results, 'Do not indent within a namespace ' + ' [runtime/indentation_namespace] [4]') + + def testNameSpaceIndentationForClass(self): + lines = ['namespace Test {', + 'void foo() { }', + ' class Test {', + ' };', + '} // namespace Test'] + + results = self.GetNamespaceResults(lines) + self.assertEquals(results, 'Do not indent within a namespace ' + ' [runtime/indentation_namespace] [4]') + + def testNameSpaceIndentationNoError(self): + lines = ['namespace Test {', + 'void foo() { }', + '} // namespace Test'] + + results = self.GetNamespaceResults(lines) + self.assertEquals(results, '') + + def testFalsePositivesNoError(self): + lines = ['namespace Test {', + 'struct OuterClass {', + ' struct NoFalsePositivesHere;', + ' struct NoFalsePositivesHere member_variable;', + '};', + '} // namespace Test'] + + results = self.GetNamespaceResults(lines) + self.assertEquals(results, '') + + # Test get line width. def testGetLineWidth(self): self.assertEquals(0, cpplint.GetLineWidth('')) @@ -296,7 +348,7 @@ class CpplintTest(CpplintTestBase): def testRemoveMultiLineCommentsFromRange(self): lines = ['a', ' /* comment ', ' * still comment', ' comment */ ', 'b'] cpplint.RemoveMultiLineCommentsFromRange(lines, 1, 4) - self.assertEquals(['a', '// dummy', '// dummy', '// dummy', 'b'], lines) + self.assertEquals(['a', '/**/', '/**/', '/**/', 'b'], lines) def testSpacesAtEndOfLine(self): self.TestLint( @@ -328,7 +380,7 @@ class CpplintTest(CpplintTestBase): 'Lines should be <= 80 characters long' ' [whitespace/line_length] [2]') self.TestLint( - '// Read https://g' + ('o' * 60) + 'gle.com/' , + '// Read https://g' + ('o' * 60) + 'gle.com/', '') self.TestLint( '// $Id: g' + ('o' * 80) + 'gle.cc#1 $', @@ -414,6 +466,10 @@ class CpplintTest(CpplintTestBase): 'int a = (int)1.0;', 'Using C-style cast. Use static_cast(...) instead' ' [readability/casting] [4]') + self.TestLint( + 'int a = (int)-1.0;', + 'Using C-style cast. Use static_cast(...) instead' + ' [readability/casting] [4]') self.TestLint( 'int *a = (int *)NULL;', 'Using C-style cast. Use reinterpret_cast(...) instead' @@ -442,6 +498,25 @@ class CpplintTest(CpplintTestBase): self.TestLint('x = alignof(int)', '') self.TestLint('alignas(int) char x[42]', '') self.TestLint('alignas(alignof(x)) char y[42]', '') + self.TestLint('void F(int (func)(int));', '') + self.TestLint('void F(int (func)(int*));', '') + self.TestLint('void F(int (Class::member)(int));', '') + self.TestLint('void F(int (Class::member)(int*));', '') + self.TestLint('void F(int (Class::member)(int), int param);', '') + self.TestLint('void F(int (Class::member)(int*), int param);', '') + + # These should not be recognized (lambda functions without arg names). + 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]') # Test taking address of casts (runtime/casting) def testRuntimeCasting(self): @@ -455,6 +530,10 @@ class CpplintTest(CpplintTestBase): ['Using C-style cast. Use reinterpret_cast(...) ' 'instead [readability/casting] [4]', error_msg]) + self.TestLint('BudgetBuckets&(BudgetWinHistory::*BucketFn)(void) const;', + '') + self.TestLint('&(*func_ptr)(arg)', '') + self.TestLint('Compute(arg, &(*func_ptr)(i, j));', '') # Alternative error message alt_error_msg = ('Are you taking an address of something dereferenced ' @@ -512,6 +591,8 @@ class CpplintTest(CpplintTestBase): self.TestLint('X operator--(int);', '') self.TestLint('X operator--(int /*unused*/) {', '') self.TestLint('MACRO(int);', '') + self.TestLint('MACRO(func(int));', '') + self.TestLint('MACRO(arg, func(int));', '') self.TestLint('void (*func)(void*);', '') self.TestLint('void Func((*func)(void*)) {}', '') @@ -978,6 +1059,14 @@ class CpplintTest(CpplintTestBase): self.TestMultiLineLint(r""" // /* comment, but not multi-line""", '') self.TestMultiLineLint(r"""/********** */""", '') + self.TestMultiLineLint(r"""/** + * Doxygen comment + */""", + '') + self.TestMultiLineLint(r"""/*! + * Doxygen comment + */""", + '') def testMultilineStrings(self): multiline_string_error_message = ( @@ -1385,10 +1474,25 @@ class CpplintTest(CpplintTestBase): self.TestLint('int F() final override;', error_message) self.TestLint('int F() final override {}', error_message) + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData( + 'foo.cc', 'cc', + ['// Copyright 2014 Your Company.', + 'struct A : virtual B {', + ' ~A() override;' + '};', + 'class C', + ' : public D,', + ' public virtual E {', + ' void Func() override;', + '}', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) + + self.TestLint('void Finalize(AnnotationProto *final) override;', '') + def testCheckDeprecated(self): - self.TestLanguageRulesCheck('foo.cc', '#include ', - 'Streams are highly discouraged.' - ' [readability/streams] [3]') self.TestLanguageRulesCheck('foo_test.cc', '#include ', '') self.TestLanguageRulesCheck('foo_unittest.cc', '#include ', '') @@ -1545,30 +1649,74 @@ class CpplintTest(CpplintTestBase): for macro_name in ( 'DISALLOW_COPY_AND_ASSIGN', 'DISALLOW_IMPLICIT_CONSTRUCTORS'): - self.TestLanguageRulesCheck( - 'some_class.h', - """%s(SomeClass); - int foo_; - };""" % macro_name, + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData( + 'foo.cc', 'cc', + ['// Copyright 2014 Your Company.', + 'class SomeClass {', + ' private:', + ' %s(SomeClass);' % macro_name, + ' int member_;', + '};', + ''], + error_collector) + self.assertEquals( ('%s should be the last thing in the class' % macro_name) + - ' [readability/constructors] [3]') - self.TestLanguageRulesCheck( - 'some_class.h', - """%s(SomeClass); - };""" % macro_name, - '') - self.TestLanguageRulesCheck( - 'some_class.h', - """%s(SomeClass); - int foo_; - } instance, *pointer_to_instance;""" % macro_name, + ' [readability/constructors] [3]', + error_collector.Results()) + + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData( + 'foo.cc', 'cc', + ['// Copyright 2014 Your Company.', + 'class OuterClass {', + ' private:', + ' struct InnerClass {', + ' private:', + ' %s(InnerClass);' % macro_name, + ' int member;', + ' };', + '};', + ''], + error_collector) + self.assertEquals( ('%s should be the last thing in the class' % macro_name) + - ' [readability/constructors] [3]') - self.TestLanguageRulesCheck( - 'some_class.h', - """%s(SomeClass); - } instance, *pointer_to_instance;""" % macro_name, - '') + ' [readability/constructors] [3]', + error_collector.Results()) + + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData( + 'foo.cc', 'cc', + ['// Copyright 2014 Your Company.', + 'class OuterClass1 {', + ' private:', + ' struct InnerClass1 {', + ' private:', + ' %s(InnerClass1);' % macro_name, + ' };', + ' %s(OuterClass1);' % macro_name, + '};', + 'struct OuterClass2 {', + ' private:', + ' class InnerClass2 {', + ' private:', + ' %s(InnerClass2);' % macro_name, + ' // comment', + ' };', + '', + ' %s(OuterClass2);' % macro_name, + '', + ' // comment', + '};', + 'void Func() {', + ' struct LocalClass {', + ' private:', + ' %s(LocalClass);' % macro_name, + ' } variable;', + '}', + ''], + error_collector) + self.assertEquals('', error_collector.Results()) # DISALLOW* macros should be in the private: section. def testMisplacedDisallowMacros(self): @@ -1947,6 +2095,20 @@ class CpplintTest(CpplintTestBase): self.TestLint('void Func(X& x) const override;', '') self.TestLint('void Func(X& x) const override {', '') + # Don't warn on out-of-line method definitions. + self.TestLint('void NS::Func(X& x) {', '') + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData( + 'foo.cc', 'cc', + ['// Copyright 2014 Your Company. All Rights Reserved.', + 'void a::b() {}', + 'void f(int& q) {}', + ''], + error_collector) + self.assertEquals( + operand_error_message % 'int& q', + error_collector.Results()) + # Other potential false positives. These need full parser # state to reproduce as opposed to just TestLint. error_collector = ErrorCollector(self.assert_) @@ -1964,8 +2126,13 @@ class CpplintTest(CpplintTestBase): ' ostream& out', ' const dense_hash_set& seq) {', '}', + 'class A {', + ' void Function(', + ' string &x) override {', + ' }', + '};', 'void Derived::Function(', - ' string &x) override {', + ' string &x) {', '}', '#define UNSUPPORTED_MASK(_mask) \\', ' if (flags & _mask) { \\', @@ -2116,6 +2283,7 @@ class CpplintTest(CpplintTestBase): self.TestLint('foo (foo)', 'Extra space before ( in function call' ' [whitespace/parens] [4]') self.TestLint('} catch (const Foo& ex) {', '') + self.TestLint('case (42):', '') self.TestLint('typedef foo (*foo)(foo)', '') self.TestLint('typedef foo (*foo12bar_)(foo)', '') self.TestLint('typedef foo (Foo::*bar)(foo)', '') @@ -2150,6 +2318,7 @@ class CpplintTest(CpplintTestBase): ' [whitespace/braces] [5]') self.TestLint('for {', '') self.TestLint('EXPECT_DEBUG_DEATH({', '') + self.TestLint('std::is_convertible{}', '') def testSemiColonAfterBraces(self): self.TestLint('if (cond) {};', @@ -2159,9 +2328,12 @@ class CpplintTest(CpplintTestBase): self.TestLint('void Func() const {};', 'You don\'t need a ; after a } [readability/braces] [4]') self.TestLint('class X {};', '') - self.TestLint('struct X {};', '') - self.TestLint('union {} x;', '') - self.TestLint('union {};', '') + for keyword in ['struct', 'union']: + for align in ['', ' alignas(16)']: + for typename in ['', ' X']: + for identifier in ['', ' x']: + self.TestLint(keyword + align + typename + ' {}' + identifier + ';', + '') self.TestLint('class X : public Y {};', '') self.TestLint('class X : public MACRO() {};', '') @@ -2345,15 +2517,20 @@ class CpplintTest(CpplintTestBase): self.TestLint('a = cast < b&& c;', space_error) # Function parameters - for head in ['void Func', 'vector Func', 'vector\nFunc', - 'Constructor', 'Constructor::Constructor', - 'operator=', 'operator =', 'operator = ']: - for body in [' {}', ' {', ';']: - self.TestMultiLineLint(head + '(A&& b)' + body, rvalue_error) - self.TestMultiLineLint(head + '(A &&b)' + body, rvalue_error) - self.TestMultiLineLint(head + '(A&&... b)' + body, rvalue_error) - self.TestMultiLineLint(head + '(A&& c)' + body, rvalue_error) - self.TestMultiLineLint(head + '(A &&c)' + body, rvalue_error) + 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) @@ -2364,6 +2541,9 @@ class CpplintTest(CpplintTestBase): 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) @@ -2397,10 +2577,41 @@ class CpplintTest(CpplintTestBase): '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;', + ' 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(), '') @@ -2658,14 +2869,15 @@ class CpplintTest(CpplintTestBase): self.TestLint('//x', 'Should have a space between // and comment' ' [whitespace/comments] [4]') self.TestLint('// x', '') + self.TestLint('///', '') + self.TestLint('/// x', '') + self.TestLint('//!', '') self.TestLint('//----', '') self.TestLint('//====', '') self.TestLint('//////', '') self.TestLint('////// x', '') - self.TestLint('/// x', '') self.TestLint('///< x', '') # After-member Doxygen comment self.TestLint('//!< x', '') # After-member Doxygen comment - self.TestLint('///', '') # Empty Doxygen comment self.TestLint('////x', 'Should have a space between // and comment' ' [whitespace/comments] [4]') self.TestLint('//}', '') @@ -3063,6 +3275,57 @@ class CpplintTest(CpplintTestBase): self.TestLint('operator,(a,b)', 'Missing space after , [whitespace/comma] [3]') + def testEqualsOperatorSpacing(self): + self.TestLint('int tmp= a;', + 'Missing spaces around = [whitespace/operators] [4]') + self.TestLint('int tmp =a;', + 'Missing spaces around = [whitespace/operators] [4]') + self.TestLint('int tmp=a;', + 'Missing spaces around = [whitespace/operators] [4]') + self.TestLint('int tmp= 7;', + 'Missing spaces around = [whitespace/operators] [4]') + self.TestLint('int tmp =7;', + 'Missing spaces around = [whitespace/operators] [4]') + self.TestLint('int tmp=7;', + 'Missing spaces around = [whitespace/operators] [4]') + self.TestLint('int* tmp=*p;', + 'Missing spaces around = [whitespace/operators] [4]') + self.TestLint('int* tmp= *p;', + 'Missing spaces around = [whitespace/operators] [4]') + self.TestMultiLineLint( + TrimExtraIndent(''' + lookahead_services_= + ::strings::Split(FLAGS_ls, ",", ::strings::SkipEmpty());'''), + 'Missing spaces around = [whitespace/operators] [4]') + self.TestLint('bool result = a>=42;', + 'Missing spaces around >= [whitespace/operators] [3]') + self.TestLint('bool result = a<=42;', + 'Missing spaces around <= [whitespace/operators] [3]') + self.TestLint('bool result = a==42;', + 'Missing spaces around == [whitespace/operators] [3]') + self.TestLint('auto result = a!=42;', + 'Missing spaces around != [whitespace/operators] [3]') + self.TestLint('int a = b!=c;', + 'Missing spaces around != [whitespace/operators] [3]') + self.TestLint('a&=42;', '') + self.TestLint('a|=42;', '') + self.TestLint('a^=42;', '') + self.TestLint('a+=42;', '') + self.TestLint('a*=42;', '') + self.TestLint('a/=42;', '') + self.TestLint('a%=42;', '') + self.TestLint('a>>=5;', '') + self.TestLint('a<<=5;', '') + + def testShiftOperatorSpacing(self): + self.TestLint('a<>b', + 'Missing spaces around >> [whitespace/operators] [3]') + self.TestLint('1<<20', '') + self.TestLint('1024>>10', '') + self.TestLint('Kernel<<<1, 2>>>()', '') + def testIndent(self): self.TestLint('static int noindent;', '') self.TestLint(' int two_space_indent;', '') @@ -3365,6 +3628,16 @@ class CpplintTest(CpplintTestBase): qux; #endif""", '') + self.TestMultiLineLint( + """void F() { + variable = [] { if (true); }; + variable = + [] { if (true); }; + Call( + [] { if (true); }, + [] { if (true); }); + }""", + '') def testTab(self): self.TestLint('\tint a;', @@ -3498,6 +3771,25 @@ class CpplintTest(CpplintTestBase): cpplint._cpplint_state.filters = old_filters cpplint._DEFAULT_FILTERS = default_filters + def testDuplicateHeader(self): + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('path/self.cc', 'cc', + ['// Copyright 2014 Your Company. All Rights Reserved.', + '#include "path/self.h"', + '#include "path/duplicate.h"', + '#include "path/duplicate.h"', + '#ifdef MACRO', + '#include "path/unique.h"', + '#else', + '#include "path/unique.h"', + '#endif', + ''], + error_collector) + self.assertEquals( + ['"path/duplicate.h" already included at path/self.cc:3 ' + '[build/include] [4]'], + error_collector.ResultList()) + def testUnnamedNamespacesInHeaders(self): self.TestLanguageRulesCheck( 'foo.h', 'namespace {', @@ -3575,26 +3867,31 @@ class CpplintTest(CpplintTestBase): ' Remove this line.' ' [build/forward_decl] [5]') - def testBuildHeaderGuard(self): - file_path = 'mydir/foo.h' - - # We can't rely on our internal stuff to get a sane path on the open source - # side of things, so just parse out the suggested header guard. This - # doesn't allow us to test the suggested header guard, but it does let us - # test all the other header tests. + def GetBuildHeaderGuardPreprocessorSymbol(self, file_path): + # Figure out the expected header guard by processing an empty file. error_collector = ErrorCollector(self.assert_) cpplint.ProcessFileData(file_path, 'h', [], error_collector) - expected_guard = '' - matcher = re.compile( - 'No \#ifndef header guard found\, suggested CPP variable is\: ([A-Z_]+) ') for error in error_collector.ResultList(): - matches = matcher.match(error) - if matches: - expected_guard = matches.group(1) - break + matched = re.search( + 'No #ifndef header guard found, suggested CPP variable is: ([A-Z_]+)', + error) + if matched is not None: + return matched.group(1) - # Make sure we extracted something for our header guard. - self.assertNotEqual(expected_guard, '') + def testBuildHeaderGuard(self): + file_path = 'mydir/foo.h' + expected_guard = self.GetBuildHeaderGuardPreprocessorSymbol(file_path) + self.assertTrue(re.search('MYDIR_FOO_H_$', expected_guard)) + + # No guard at all: expect one error. + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData(file_path, 'h', [], error_collector) + self.assertEquals( + 1, + error_collector.ResultList().count( + 'No #ifndef header guard found, suggested CPP variable is: %s' + ' [build/header_guard] [5]' % expected_guard), + error_collector.ResultList()) # No header guard, but the error is suppressed. error_collector = ErrorCollector(self.assert_) @@ -3622,7 +3919,7 @@ class CpplintTest(CpplintTestBase): self.assertEquals( 1, error_collector.ResultList().count( - 'No #define header guard found, suggested CPP variable is: %s' + 'No #ifndef header guard found, suggested CPP variable is: %s' ' [build/header_guard] [5]' % expected_guard), error_collector.ResultList()) @@ -3635,7 +3932,7 @@ class CpplintTest(CpplintTestBase): self.assertEquals( 1, error_collector.ResultList().count( - '#ifndef and #define don\'t match, suggested CPP variable is: %s' + 'No #ifndef header guard found, suggested CPP variable is: %s' ' [build/header_guard] [5]' % expected_guard), error_collector.ResultList()) @@ -3643,7 +3940,8 @@ class CpplintTest(CpplintTestBase): error_collector = ErrorCollector(self.assert_) cpplint.ProcessFileData(file_path, 'h', ['#ifndef %s' % expected_guard, - '#define %s' % expected_guard], + '#define %s' % expected_guard, + ''], error_collector) self.assertEquals( 1, @@ -3764,7 +4062,9 @@ class CpplintTest(CpplintTestBase): # Special case for flymake for test_file in ['mydir/foo_flymake.h', 'mydir/.flymake/foo.h']: error_collector = ErrorCollector(self.assert_) - cpplint.ProcessFileData(test_file, 'h', [], error_collector) + cpplint.ProcessFileData(test_file, 'h', + ['// Copyright 2014 Your Company.', ''], + error_collector) self.assertEquals( 1, error_collector.ResultList().count( @@ -4062,7 +4362,6 @@ class CleansedLinesTest(unittest.TestCase): 'Line 4 /* Comment test */', 'Line 5 "foo"'] - clean_lines = cpplint.CleansedLines(lines) self.assertEquals(lines, clean_lines.raw_lines) self.assertEquals(5, clean_lines.NumLines()) @@ -4226,11 +4525,11 @@ class OrderOfIncludesTest(CpplintTestBase): def testRegression(self): def Format(includes): include_list = [] - for header_path in includes: - if header_path: - include_list.append('#include %s\n' % header_path) + for item in includes: + if item.startswith('"') or item.startswith('<'): + include_list.append('#include %s\n' % item) else: - include_list.append('\n') + include_list.append(item + '\n') return ''.join(include_list) # Test singleton cases first. @@ -4313,6 +4612,9 @@ class OrderOfIncludesTest(CpplintTestBase): '', '"base/google.h"', '', + '"b/c.h"', + '', + 'MACRO', '"a/b.h"']), '') self.TestLanguageRulesCheck('a/a.cc', @@ -5139,7 +5441,7 @@ def tearDown(): ErrorCollector(None).VerifyAllCategoriesAreSeen() except NameError: # If nobody set the global _run_verifyallcategoriesseen, then - # we assume we shouldn't run the test + # we assume we should silently not run the test pass