From 8a95ecca274abfd187a005e71a94acb0ff4565b9 Mon Sep 17 00:00:00 2001 From: "erg@google.com" Date: Thu, 8 Sep 2011 00:45:54 +0000 Subject: [PATCH] Update to #179: - Suggest "const_cast" when encountering "(char*)". - Warn on template arguments to make_pair(). - Require if pair<> is seen now that doesn't include it. - Warn on lack of "explicit" keyword on single argument inline constructors. - Better check for hanging ')' when closing function calls. - Don't warn on: 'int v[1][3] = {{1, 2, 3}};' - Allow function calls as the first argument to printf(). Review URL: http://codereview.appspot.com/4974066 --- cpplint/cpplint.py | 313 +++++++++++++++++----- cpplint/cpplint_unittest.py | 513 +++++++++++++++++++++++------------- 2 files changed, 585 insertions(+), 241 deletions(-) diff --git a/cpplint/cpplint.py b/cpplint/cpplint.py index ef5e80f..19308c1 100755 --- a/cpplint/cpplint.py +++ b/cpplint/cpplint.py @@ -150,6 +150,7 @@ _ERROR_CATEGORIES = [ 'build/class', 'build/deprecated', 'build/endif_comment', + 'build/explicit_make_pair', 'build/forward_decl', 'build/header_guard', 'build/include', @@ -210,11 +211,11 @@ _ERROR_CATEGORIES = [ # flag. By default all errors are on, so only add here categories that should be # off by default (i.e., categories that must be enabled by the --filter= flags). # All entries here should start with a '-' or '+', as in the --filter= flag. -_DEFAULT_FILTERS = [ '-build/include_alpha' ] +_DEFAULT_FILTERS = ['-build/include_alpha'] # We used to check for high-bit characters, but after much discussion we # decided those were OK, as long as they were in UTF-8 and didn't represent -# hard-coded international strings, which belong in a seperate i18n file. +# hard-coded international strings, which belong in a separate i18n file. # Headers that we consider STL headers. _STL_HEADERS = frozenset([ @@ -310,9 +311,9 @@ def ParseNolintSuppressions(filename, raw_line, linenum, error): error: function, an error handler. """ # FIXME(adonovan): "NOLINT(" is misparsed as NOLINT(*). - m = _RE_SUPPRESSION.search(raw_line) - if m: - category = m.group(1) + matched = _RE_SUPPRESSION.search(raw_line) + if matched: + category = matched.group(1) if category in (None, '(*)'): # => "suppress all" _error_suppressions.setdefault(None, set()).add(linenum) else: @@ -322,7 +323,7 @@ def ParseNolintSuppressions(filename, raw_line, linenum, error): _error_suppressions.setdefault(category, set()).add(linenum) else: error(filename, linenum, 'readability/nolint', 5, - 'Unknown NOLINT error category: %s' % category) + 'Unknown NOLINT error category: %s' % category) def ResetNolintSuppressions(): @@ -404,7 +405,7 @@ class _IncludeState(dict): self._last_header = '' def CanonicalizeAlphabeticalOrder(self, header_path): - """Returns a path canonicalized for alphabetical comparisson. + """Returns a path canonicalized for alphabetical comparison. - replaces "-" with "_" so they both cmp the same. - removes '-inl' since we don't require them to be after the main header. @@ -662,7 +663,7 @@ class _FunctionState(object): self.current_function, self.lines_in_function, trigger)) def End(self): - """Stop analizing function body.""" + """Stop analyzing function body.""" self.in_a_function = False @@ -760,8 +761,7 @@ class FileInfo: def _ShouldPrintError(category, confidence, linenum): - """Returns true iff confidence >= verbose, category passes - filter and is not NOLINT-suppressed.""" + """If confidence >= verbose, category passes filter and is not suppressed.""" # There are three ways we might decide not to print an error message: # a "NOLINT(category)" comment appears in the source, @@ -966,7 +966,7 @@ class CleansedLines(object): def CloseExpression(clean_lines, linenum, pos): """If input points to ( or { or [, finds the position that closes it. - If lines[linenum][pos] points to a '(' or '{' or '[', finds the the + If lines[linenum][pos] points to a '(' or '{' or '[', finds the linenum/pos that correspond to the closing of the expression. Args: @@ -1248,7 +1248,7 @@ def CheckInvalidIncrement(filename, clean_lines, linenum, error): class _ClassInfo(object): """Stores information about a class.""" - def __init__(self, name, linenum): + def __init__(self, name, clean_lines, linenum): self.name = name self.linenum = linenum self.seen_open_brace = False @@ -1257,6 +1257,20 @@ class _ClassInfo(object): self.has_virtual_destructor = False self.brace_depth = 0 + # Try to find the end of the class. This will be confused by things like: + # class A { + # } *x = { ... + # + # But it's still good enough for CheckSectionSpacing. + self.last_line = 0 + depth = 0 + for i in range(linenum, clean_lines.NumLines()): + line = clean_lines.lines[i] + depth += line.count('{') - line.count('}') + if not depth: + self.last_line = i + break + class _ClassState(object): """Holds the current state of the parse relating to class declarations. @@ -1383,9 +1397,11 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, # class LOCKABLE API Object { # }; class_decl_match = Match( - r'\s*(template\s*<[\w\s<>,:]*>\s*)?(class|struct)\s+([A-Z_]+\s+)*(\w+(::\w+)*)', line) + r'\s*(template\s*<[\w\s<>,:]*>\s*)?' + '(class|struct)\s+([A-Z_]+\s+)*(\w+(::\w+)*)', line) if class_decl_match: - classinfo_stack.append(_ClassInfo(class_decl_match.group(4), linenum)) + classinfo_stack.append(_ClassInfo( + class_decl_match.group(4), clean_lines, linenum)) # Everything else in this function uses the top of the stack if it's # not empty. @@ -1415,12 +1431,12 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, # Look for single-argument constructors that aren't marked explicit. # Technically a valid construct, but against style. - args = Match(r'(?\s*)?&' % re.escape(base_classname), args.group(1).strip())): error(filename, linenum, 'runtime/explicit', 5, 'Single-argument constructors should be marked explicit.') @@ -1511,8 +1527,14 @@ def CheckSpacingForFunctionCall(filename, line, linenum, error): # If the ) is followed only by a newline or a { + newline, assume it's # part of a control statement (if/while/etc), and don't complain if Search(r'[^)]\s+\)\s*[^{\s]', fncall): - error(filename, linenum, 'whitespace/parens', 2, - 'Extra space before )') + # If the closing parenthesis is preceded by only whitespaces, + # try to give a more descriptive error message. + if Search(r'^\s+\)', fncall): + error(filename, linenum, 'whitespace/parens', 2, + 'Closing ) should be moved to the previous line') + else: + error(filename, linenum, 'whitespace/parens', 2, + 'Extra space before )') def IsBlankLine(line): @@ -1543,7 +1565,7 @@ def CheckForFunctionLengths(filename, clean_lines, linenum, Trivial bodies are unchecked, so constructors with huge initializer lists may be missed. Blank/comment lines are not counted so as to avoid encouraging the removal - of vertical space and commments just to get through a lint check. + of vertical space and comments just to get through a lint check. NOLINT *on the last line of a function* disables this check. Args: @@ -1639,8 +1661,8 @@ def CheckSpacing(filename, clean_lines, linenum, error): Things we check for: spaces around operators, spaces after if/for/while/switch, no spaces around parens in function calls, two spaces between code and comment, don't start a block with a blank - line, don't end a function with a blank line, don't have too many - blank lines in a row. + line, don't end a function with a blank line, don't add a blank line + after public/protected/private, don't have too many blank lines in a row. Args: filename: The name of the current file. @@ -1667,7 +1689,7 @@ def CheckSpacing(filename, clean_lines, linenum, error): and prev_line[:prevbrace].find('namespace') == -1): # OK, we have a blank line at the start of a code block. Before we # complain, we check if it is an exception to the rule: The previous - # non-empty line has the paramters of a function header that are indented + # non-empty line has the parameters of a function header that are indented # 4 spaces (because they did not fit in a 80 column line when placed on # the same line as the function name). We also check for the case where # the previous line is indented 6 spaces, which may happen when the @@ -1718,6 +1740,11 @@ def CheckSpacing(filename, clean_lines, linenum, error): error(filename, linenum, 'whitespace/blank_line', 3, 'Blank line at the end of a code block. Is this needed?') + matched = Match(r'\s*(public|protected|private):', prev_line) + if matched: + error(filename, linenum, 'whitespace/blank_line', 3, + 'Do not leave a blank line after "%s:"' % matched.group(1)) + # Next, we complain if there's a comment too near the text commentpos = line.find('//') if commentpos != -1: @@ -1838,10 +1865,11 @@ def CheckSpacing(filename, clean_lines, linenum, error): # Next we will look for issues with function calls. CheckSpacingForFunctionCall(filename, line, linenum, error) - # Except after an opening paren, you should have spaces before your braces. - # And since you should never have braces at the beginning of a line, this is - # an easy test. - if Search(r'[^ (]{', line): + # Except after an opening paren, or after another opening brace (in case of + # an initializer list, for instance), you should have spaces before your + # braces. And since you should never have braces at the beginning of a line, + # this is an easy test. + if Search(r'[^ ({]{', line): error(filename, linenum, 'whitespace/braces', 5, 'Missing space before {') @@ -1873,6 +1901,58 @@ def CheckSpacing(filename, clean_lines, linenum, error): 'statement, use { } instead.') +def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error): + """Checks for additional blank line issues related to sections. + + Currently the only thing checked here is blank line before protected/private. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + class_info: A _ClassInfo objects. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + # Skip checks if the class is small, where small means 25 lines or less. + # 25 lines seems like a good cutoff since that's the usual height of + # terminals, and any class that can't fit in one screen can't really + # be considered "small". + # + # Also skip checks if we are on the first line. This accounts for + # classes that look like + # class Foo { public: ... }; + # + # If we didn't find the end of the class, last_line would be zero, + # and the check will be skipped by the first condition. + if (class_info.last_line - class_info.linenum <= 24 or + linenum <= class_info.linenum): + return + + matched = Match(r'\s*(public|protected|private):', clean_lines.lines[linenum]) + if matched: + # Issue warning if the line before public/protected/private was + # not a blank line, but don't do this if the previous line contains + # "class" or "struct". This can happen two ways: + # - We are at the beginning of the class. + # - We are forward-declaring an inner class that is semantically + # private, but needed to be public for implementation reasons. + prev_line = clean_lines.lines[linenum - 1] + if (not IsBlankLine(prev_line) and + not Search(r'\b(class|struct)\b', prev_line)): + # Try a bit harder to find the beginning of the class. This is to + # account for multi-line base-specifier lists, e.g.: + # class Derived + # : public Base { + end_class_head = class_info.linenum + for i in range(class_info.linenum, linenum): + if Search(r'\{\s*$', clean_lines.lines[i]): + end_class_head = i + break + if end_class_head < linenum - 1: + error(filename, linenum, 'whitespace/blank_line', 3, + '"%s:" should be preceded by a blank line' % matched.group(1)) + + def GetPreviousNonBlankLine(clean_lines, linenum): """Return the most recent non-blank line and its line number. @@ -2050,17 +2130,18 @@ def GetLineWidth(line): """ if isinstance(line, unicode): width = 0 - for c in unicodedata.normalize('NFC', line): - if unicodedata.east_asian_width(c) in ('W', 'F'): + for uc in unicodedata.normalize('NFC', line): + if unicodedata.east_asian_width(uc) in ('W', 'F'): width += 2 - elif not unicodedata.combining(c): + elif not unicodedata.combining(uc): width += 1 return width else: return len(line) -def CheckStyle(filename, clean_lines, linenum, file_extension, error): +def CheckStyle(filename, clean_lines, linenum, file_extension, class_state, + error): """Checks rules from the 'C++ style rules' section of cppguide.html. Most of these rules are hard to test (naming, comment style), but we @@ -2160,6 +2241,9 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, error): CheckBraces(filename, clean_lines, linenum, error) CheckSpacing(filename, clean_lines, linenum, error) CheckCheck(filename, clean_lines, linenum, error) + if class_state and class_state.classinfo_stack: + CheckSectionSpacing(filename, clean_lines, + class_state.classinfo_stack[-1], linenum, error) _RE_PATTERN_INCLUDE_NEW_STYLE = re.compile(r'#include +"[^/]+\.h"') @@ -2345,6 +2429,63 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): error(filename, linenum, 'readability/streams', 3, 'Streams are highly discouraged.') + +def _GetTextInside(text, start_pattern): + """Retrieves all the text between matching open and close parentheses. + + Given a string of lines and a regular expression string, retrieve all the text + following the expression and between opening punctuation symbols like + (, [, or {, and the matching close-punctuation symbol. This properly nested + occurrences of the punctuations, so for the text like + printf(a(), b(c())); + a call to _GetTextInside(text, r'printf\(') will return 'a(), b(c())'. + start_pattern must match string having an open punctuation symbol at the end. + + Args: + text: The lines to extract text. Its comments and strings must be elided. + It can be single line and can span multiple lines. + start_pattern: The regexp string indicating where to start extracting + the text. + Returns: + The extracted text. + None if either the opening string or ending punctuation could not be found. + """ + # TODO(sugawarayu): Audit cpplint.py to see what places could be profitably + # rewritten to use _GetTextInside (and use inferior regexp matching today). + + # Give opening punctuations to get the matching close-punctuations. + matching_punctuation = {'(': ')', '{': '}', '[': ']'} + closing_punctuation = set(matching_punctuation.itervalues()) + + # Find the position to start extracting text. + match = re.search(start_pattern, text, re.M) + if not match: # start_pattern not found in text. + return None + start_position = match.end(0) + + assert start_position > 0, ( + 'start_pattern must ends with an opening punctuation.') + assert text[start_position - 1] in matching_punctuation, ( + 'start_pattern must ends with an opening punctuation.') + # Stack of closing punctuations we expect to have in text after position. + punctuation_stack = [matching_punctuation[text[start_position - 1]]] + position = start_position + while punctuation_stack and position < len(text): + if text[position] == punctuation_stack[-1]: + punctuation_stack.pop() + elif text[position] in closing_punctuation: + # A closing punctuation without matching opening punctuations. + return None + elif text[position] in matching_punctuation: + punctuation_stack.append(matching_punctuation[text[position]]) + position += 1 + if punctuation_stack: + # Opening punctuations left without matching close-punctuations. + return None + # punctuations match. + return text[start_position:position - 1] + + def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, error): """Checks rules from the 'C++ language rules' section of cppguide.html. @@ -2390,7 +2531,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, # These are complicated re's. They try to capture the following: # paren (for fn-prototype start), typename, &, varname. For the const # version, we're willing for const to be before typename or after - # Don't check the implemention on same line. + # Don't check the implementation on same line. fnline = line.split('{', 1)[0] if (len(re.findall(r'\([^()]*\b(?:[\w:]|<[^()]*>)+(\s?&|&\s?)\w+', fnline)) > len(re.findall(r'\([^()]*\bconst\s+(?:typename\s+)?(?:struct\s+)?' @@ -2430,11 +2571,19 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], 'static_cast', - r'\((int|float|double|bool|char|u?int(16|32|64))\)', - error) - # This doesn't catch all cases. Consider (const char * const)"hello". - CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], - 'reinterpret_cast', r'\((\w+\s?\*+\s?)\)', error) + r'\((int|float|double|bool|char|u?int(16|32|64))\)', error) + + # This doesn't catch all cases. Consider (const char * const)"hello". + # + # (char *) "foo" should always be a const_cast (reinterpret_cast won't + # compile). + if CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], + 'const_cast', r'\((char\s?\*+\s?)\)\s*"', error): + pass + else: + # Check pointer casts for other than string constants + CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], + 'reinterpret_cast', r'\((\w+\s?\*+\s?)\)', error) # In addition, we look for people taking the address of a cast. This # is dangerous -- casts can assign to temporaries, so the pointer doesn't @@ -2533,11 +2682,19 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, # Check for potential format string bugs like printf(foo). # We constrain the pattern not to pick things like DocidForPrintf(foo). # Not perfect but it can catch printf(foo.c_str()) and printf(foo->c_str()) - match = re.search(r'\b((?:string)?printf)\s*\(([\w.\->()]+)\)', line, re.I) - if match: - error(filename, linenum, 'runtime/printf', 4, - 'Potential format string bug. Do %s("%%s", %s) instead.' - % (match.group(1), match.group(2))) + # TODO(sugawarayu): Catch the following case. Need to change the calling + # convention of the whole function to process multiple line to handle it. + # printf( + # boy_this_is_a_really_long_variable_that_cannot_fit_on_the_prev_line); + printf_args = _GetTextInside(line, r'(?i)\b(string)?printf\s*\(') + if printf_args: + match = Match(r'([\w.\->()]+)$', printf_args) + if match: + function_name = re.search(r'\b((?:string)?printf)\s*\(', + line, re.I).group(1) + error(filename, linenum, 'runtime/printf', 4, + 'Potential format string bug. Do %s("%%s", %s) instead.' + % (function_name, match.group(1))) # Check for potential memset bugs like memset(buf, sizeof(buf), 0). match = Search(r'memset\s*\(([^,]*),\s*([^,]*),\s*0\s*\)', line) @@ -2579,7 +2736,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, if Match(r'(.+::)?[A-Z][A-Z0-9_]*', tok): continue # A catch all for tricky sizeof cases, including 'sizeof expression', # 'sizeof(*type)', 'sizeof(const type)', 'sizeof(struct StructName)' - # requires skipping the next token becasue we split on ' ' and '*'. + # requires skipping the next token because we split on ' ' and '*'. if tok.startswith('sizeof'): skip_next = True continue @@ -2600,7 +2757,13 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, line) if match and linenum + 1 < clean_lines.NumLines(): next_line = clean_lines.elided[linenum + 1] - if not Search(r'^\s*};', next_line): + # We allow some, but not all, declarations of variables to be present + # in the statement that defines the class. The [\w\*,\s]* fragment of + # the regular expression below allows users to declare instances of + # the class or pointers to instances, but not less common types such + # as function pointers or arrays. It's a tradeoff between allowing + # reasonable code and avoiding trying to parse more C++ using regexps. + if not Search(r'^\s*}[\w\*,\s]*;', next_line): error(filename, linenum, 'readability/constructors', 3, match.group(1) + ' should be the last thing in the class') @@ -2628,20 +2791,24 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern, line: The line of code to check. raw_line: The raw line of code to check, with comments. cast_type: The string for the C++ cast to recommend. This is either - reinterpret_cast or static_cast, depending. + reinterpret_cast, static_cast, or const_cast, depending. pattern: The regular expression used to find C-style casts. error: The function to call with any errors found. + + Returns: + True if an error was emitted. + False otherwise. """ match = Search(pattern, line) if not match: - return + return False # e.g., sizeof(int) sizeof_match = Match(r'.*sizeof\s*$', line[0:match.start(1) - 1]) if sizeof_match: error(filename, linenum, 'runtime/sizeof', 1, 'Using sizeof(type). Use sizeof(varname) instead if possible') - return + return True remainder = line[match.end(0):] @@ -2665,13 +2832,15 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern, '/*' not in raw_line)): error(filename, linenum, 'readability/function', 3, 'All parameters should be named in a function') - return + return True # At this point, all that should be left is actual casts. error(filename, linenum, 'readability/casting', 4, 'Using C-style cast. Use %s<%s>(...) instead' % (cast_type, match.group(1))) + return True + _HEADERS_CONTAINING_TEMPLATES = ( ('', ('deque',)), @@ -2710,11 +2879,6 @@ _HEADERS_CONTAINING_TEMPLATES = ( ('', ('slist',)), ) -_HEADERS_ACCEPTED_BUT_NOT_PROMOTED = { - # We can trust with reasonable confidence that map gives us pair<>, too. - 'pair<>': ('map', 'multimap', 'hash_map', 'hash_multimap') -} - _RE_PATTERN_STRING = re.compile(r'\bstring\b') _re_pattern_algorithm_header = [] @@ -2847,11 +3011,11 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, continue # String is special -- it is a non-templatized type in STL. - m = _RE_PATTERN_STRING.search(line) - if m: + matched = _RE_PATTERN_STRING.search(line) + if matched: # Don't warn about strings in non-STL namespaces: # (We check only the first match per line; good enough.) - prefix = line[:m.start()] + prefix = line[:matched.start()] if prefix.endswith('std::') or not prefix.endswith('::'): required[''] = (linenum, 'string') @@ -2889,7 +3053,8 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, # include_state is modified during iteration, so we iterate over a copy of # the keys. - for header in include_state.keys(): #NOLINT + header_keys = include_state.keys() + for header in header_keys: (same_module, common_path) = FilesBelongToSameModule(abs_filename, header) fullpath = common_path + header if same_module and UpdateIncludeState(fullpath, include_state, io): @@ -2906,16 +3071,37 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, # All the lines have been processed, report the errors found. for required_header_unstripped in required: template = required[required_header_unstripped][1] - if template in _HEADERS_ACCEPTED_BUT_NOT_PROMOTED: - headers = _HEADERS_ACCEPTED_BUT_NOT_PROMOTED[template] - if [True for header in headers if header in include_state]: - continue if required_header_unstripped.strip('<>"') not in include_state: error(filename, required[required_header_unstripped][0], 'build/include_what_you_use', 4, 'Add #include ' + required_header_unstripped + ' for ' + template) +_RE_PATTERN_EXPLICIT_MAKEPAIR = re.compile(r'\bmake_pair\s*<') + + +def CheckMakePairUsesDeduction(filename, clean_lines, linenum, error): + """Check that make_pair's template arguments are deduced. + + G++ 4.6 in C++0x mode fails badly if make_pair's template arguments are + specified explicitly, and such use isn't intended in any case. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + raw = clean_lines.raw_lines + line = raw[linenum] + match = _RE_PATTERN_EXPLICIT_MAKEPAIR.search(line) + if match: + error(filename, linenum, 'build/explicit_make_pair', + 4, # 4 = high confidence + 'Omit template arguments from make_pair OR use pair directly OR' + ' if appropriate, construct a pair directly') + + def ProcessLine(filename, file_extension, clean_lines, line, include_state, function_state, class_state, error, extra_check_functions=[]): @@ -2941,13 +3127,14 @@ def ProcessLine(filename, file_extension, ParseNolintSuppressions(filename, raw_lines[line], line, error) CheckForFunctionLengths(filename, clean_lines, line, function_state, error) CheckForMultilineCommentsAndStrings(filename, clean_lines, line, error) - CheckStyle(filename, clean_lines, line, file_extension, error) + CheckStyle(filename, clean_lines, line, file_extension, class_state, error) CheckLanguage(filename, clean_lines, line, file_extension, include_state, error) CheckForNonStandardConstructs(filename, clean_lines, line, class_state, error) CheckPosixThreading(filename, clean_lines, line, error) CheckInvalidIncrement(filename, clean_lines, line, error) + CheckMakePairUsesDeduction(filename, clean_lines, line, error) for check_fn in extra_check_functions: check_fn(filename, clean_lines, line, error) @@ -2959,7 +3146,7 @@ def ProcessFileData(filename, file_extension, lines, error, filename: Filename of the file that is being processed. file_extension: The extension (dot not included) of the file. lines: An array of strings, each representing a line of the file, with the - last element being empty if the file is termined with a newline. + last element being empty if the file is terminated with a newline. error: A callable to which errors are reported, which takes 4 arguments: filename, line number, error level, and message extra_check_functions: An array of additional check functions that will be @@ -3055,7 +3242,7 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): ProcessFileData(filename, file_extension, lines, Error, extra_check_functions) if carriage_return_found and os.linesep != '\r\n': - # Use 0 for linenum since outputing only one error for potentially + # Use 0 for linenum since outputting only one error for potentially # several lines. Error(filename, 0, 'whitespace/newline', 1, 'One or more unexpected \\r (^M) found;' diff --git a/cpplint/cpplint_unittest.py b/cpplint/cpplint_unittest.py index e27573a..9903bc9 100755 --- a/cpplint/cpplint_unittest.py +++ b/cpplint/cpplint_unittest.py @@ -134,7 +134,8 @@ class CpplintTestBase(unittest.TestCase): lines = cpplint.CleansedLines(lines) class_state = cpplint._ClassState() for i in xrange(lines.NumLines()): - cpplint.CheckStyle('foo.h', lines, i, 'h', error_collector) + cpplint.CheckStyle('foo.h', lines, i, 'h', class_state, + error_collector) cpplint.CheckForNonStandardConstructs('foo.h', lines, i, class_state, error_collector) class_state.CheckFinished('foo.h', error_collector) @@ -243,6 +244,29 @@ class CpplintTest(CpplintTestBase): self.assertEquals(10, cpplint.GetLineWidth(u'x' * 10)) self.assertEquals(16, cpplint.GetLineWidth(u'都|道|府|県|支庁')) + def testGetTextInside(self): + self.assertEquals('', cpplint._GetTextInside('fun()', r'fun\(')) + self.assertEquals('x, y', cpplint._GetTextInside('f(x, y)', r'f\(')) + self.assertEquals('a(), b(c())', cpplint._GetTextInside( + 'printf(a(), b(c()))', r'printf\(')) + self.assertEquals('x, y{}', cpplint._GetTextInside('f[x, y{}]', r'f\[')) + self.assertEquals(None, cpplint._GetTextInside('f[a, b(}]', r'f\[')) + self.assertEquals(None, cpplint._GetTextInside('f[x, y]', r'f\(')) + self.assertEquals('y, h(z, (a + b))', cpplint._GetTextInside( + 'f(x, g(y, h(z, (a + b))))', r'g\(')) + self.assertEquals('f(f(x))', cpplint._GetTextInside('f(f(f(x)))', r'f\(')) + # Supports multiple lines. + self.assertEquals('\n return loop(x);\n', + cpplint._GetTextInside( + 'int loop(int x) {\n return loop(x);\n}\n', r'\{')) + # '^' matches the beggining of each line. + self.assertEquals('x, y', + cpplint._GetTextInside( + '#include "inl.h" // skip #define\n' + '#define A2(x, y) a_inl_(x, y, __LINE__)\n' + '#define A(x) a_inl_(x, "", __LINE__)\n', + r'^\s*#define\s*\w+\(')) + def testFindNextMultiLineCommentStart(self): self.assertEquals(1, cpplint.FindNextMultiLineCommentStart([''], 0)) @@ -311,7 +335,7 @@ class CpplintTest(CpplintTestBase): ' [readability/casting] [4]', 'Use int16/int64/etc, rather than the C type long' ' [runtime/int] [4]', - ]) + ]) # One category of error suppressed: self.TestLint( 'long a = (int64) 65; // NOLINT(runtime/int)', @@ -323,14 +347,14 @@ class CpplintTest(CpplintTestBase): # Malformed NOLINT directive: self.TestLint( 'long a = 65; // NOLINT(foo)', - ['Unknown NOLINT error category: foo' - ' [readability/nolint] [5]', - 'Use int16/int64/etc, rather than the C type long [runtime/int] [4]', - ]) + ['Unknown NOLINT error category: foo' + ' [readability/nolint] [5]', + 'Use int16/int64/etc, rather than the C type long [runtime/int] [4]', + ]) # Irrelevant NOLINT directive has no effect: self.TestLint( 'long a = 65; // NOLINT(readability/casting)', - 'Use int16/int64/etc, rather than the C type long' + 'Use int16/int64/etc, rather than the C type long' ' [runtime/int] [4]') @@ -467,6 +491,19 @@ class CpplintTest(CpplintTestBase): 'Using deprecated casting style. ' 'Use static_cast(...) instead' ' [readability/casting] [4]') + + self.TestLint( + '(char *) "foo"', + 'Using C-style cast. ' + 'Use const_cast(...) instead' + ' [readability/casting] [4]') + + self.TestLint( + '(int*)foo', + 'Using C-style cast. ' + 'Use reinterpret_cast(...) instead' + ' [readability/casting] [4]') + # Checks for false positives... self.TestLint( 'int a = int(); // Constructor, o.k.', @@ -554,74 +591,77 @@ class CpplintTest(CpplintTestBase): def testIncludeWhatYouUse(self): self.TestIncludeWhatYouUse( - '''#include + """#include std::vector foo; - ''', + """, '') self.TestIncludeWhatYouUse( - '''#include + """#include std::pair foo; - ''', - '') - self.TestIncludeWhatYouUse( - '''#include - std::pair foo; - ''', - '') - self.TestIncludeWhatYouUse( - '''#include - std::pair foo; - ''', - '') - self.TestIncludeWhatYouUse( - '''#include - std::pair foo; - ''', - '') - self.TestIncludeWhatYouUse( - '''#include - DECLARE_string(foobar); - ''', - '') - self.TestIncludeWhatYouUse( - '''#include - DEFINE_string(foobar, "", ""); - ''', - '') - self.TestIncludeWhatYouUse( - '''#include - std::pair foo; - ''', + """, 'Add #include for pair<>' ' [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''#include "base/foobar.h" + """#include + std::pair foo; + """, + 'Add #include for pair<>' + ' [build/include_what_you_use] [4]') + self.TestIncludeWhatYouUse( + """#include + std::pair foo; + """, + 'Add #include for pair<>' + ' [build/include_what_you_use] [4]') + self.TestIncludeWhatYouUse( + """#include + std::pair foo; + """, + '') + self.TestIncludeWhatYouUse( + """#include + DECLARE_string(foobar); + """, + '') + self.TestIncludeWhatYouUse( + """#include + DEFINE_string(foobar, "", ""); + """, + '') + self.TestIncludeWhatYouUse( + """#include + std::pair foo; + """, + 'Add #include for pair<>' + ' [build/include_what_you_use] [4]') + self.TestIncludeWhatYouUse( + """#include "base/foobar.h" std::vector foo; - ''', + """, 'Add #include for vector<>' ' [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''#include + """#include std::set foo; - ''', + """, 'Add #include for set<>' ' [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''#include "base/foobar.h" + """#include "base/foobar.h" hash_map foobar; - ''', + """, 'Add #include for hash_map<>' ' [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''#include "base/foobar.h" + """#include "base/foobar.h" bool foobar = std::less(0,1); - ''', + """, 'Add #include for less<>' ' [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''#include "base/foobar.h" + """#include "base/foobar.h" bool foobar = min(0,1); - ''', + """, 'Add #include for min [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( 'void a(const string &foobar);', @@ -633,55 +673,55 @@ class CpplintTest(CpplintTestBase): 'void a(const my::string &foobar);', '') # Avoid false positives on strings in other namespaces. self.TestIncludeWhatYouUse( - '''#include "base/foobar.h" + """#include "base/foobar.h" bool foobar = swap(0,1); - ''', + """, 'Add #include for swap [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''#include "base/foobar.h" + """#include "base/foobar.h" bool foobar = transform(a.begin(), a.end(), b.start(), Foo); - ''', + """, 'Add #include for transform ' '[build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''#include "base/foobar.h" + """#include "base/foobar.h" bool foobar = min_element(a.begin(), a.end()); - ''', + """, 'Add #include for min_element ' '[build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''foo->swap(0,1); + """foo->swap(0,1); foo.swap(0,1); - ''', + """, '') self.TestIncludeWhatYouUse( - '''#include + """#include void a(const std::multimap &foobar); - ''', + """, 'Add #include for multimap<>' ' [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''#include + """#include void a(const std::priority_queue &foobar); - ''', + """, '') self.TestIncludeWhatYouUse( - '''#include + """#include #include #include #include "base/basictypes.h" #include "base/port.h" - vector hajoa;''', '') + vector hajoa;""", '') self.TestIncludeWhatYouUse( - '''#include + """#include int i = numeric_limits::max() - ''', + """, 'Add #include for numeric_limits<>' ' [build/include_what_you_use] [4]') self.TestIncludeWhatYouUse( - '''#include + """#include int i = numeric_limits::max() - ''', + """, '') # Test the UpdateIncludeState code path. @@ -694,8 +734,8 @@ class CpplintTest(CpplintTestBase): mock_header_contents = ['#include '] message = self.PerformIncludeWhatYouUse( - '''#include "blah/a.h" - std::set foo;''', + """#include "blah/a.h" + std::set foo;""", filename='blah/a.cc', io=MockIo(mock_header_contents)) self.assertEquals(message, '') @@ -704,29 +744,29 @@ class CpplintTest(CpplintTestBase): # a temporary file generated by Emacs's flymake. mock_header_contents = [''] message = self.PerformIncludeWhatYouUse( - '''#include "blah/a.h" - std::set foo;''', + """#include "blah/a.h" + std::set foo;""", filename='blah/a_flymake.cc', io=MockIo(mock_header_contents)) self.assertEquals(message, 'Add #include for set<> ' - '[build/include_what_you_use] [4]') + '[build/include_what_you_use] [4]') # If there's just a cc and the header can't be found then it's ok. message = self.PerformIncludeWhatYouUse( - '''#include "blah/a.h" - std::set foo;''', + """#include "blah/a.h" + std::set foo;""", filename='blah/a.cc') self.assertEquals(message, '') # Make sure we find the headers with relative paths. mock_header_contents = [''] message = self.PerformIncludeWhatYouUse( - '''#include "%s/a.h" - std::set foo;''' % os.path.basename(os.getcwd()), + """#include "%s/a.h" + std::set foo;""" % os.path.basename(os.getcwd()), filename='a.cc', io=MockIo(mock_header_contents)) self.assertEquals(message, 'Add #include for set<> ' - '[build/include_what_you_use] [4]') + '[build/include_what_you_use] [4]') def testFilesBelongToSameModule(self): f = cpplint.FilesBelongToSameModule @@ -770,22 +810,22 @@ class CpplintTest(CpplintTestBase): def testMultiLineComments(self): # missing explicit is bad self.TestMultiLineLint( - r'''int a = 0; + r"""int a = 0; /* multi-liner class Foo { Foo(int f); // should cause a lint warning in code } - */ ''', + */ """, '') self.TestMultiLineLint( - r'''/* int a = 0; multi-liner - static const int b = 0;''', + r"""/* int a = 0; multi-liner + static const int b = 0;""", 'Could not find end of multi-line comment' ' [readability/multiline_comment] [5]') - self.TestMultiLineLint(r''' /* multi-line comment''', + self.TestMultiLineLint(r""" /* multi-line comment""", 'Could not find end of multi-line comment' ' [readability/multiline_comment] [5]') - self.TestMultiLineLint(r''' // /* comment, but not multi-line''', '') + self.TestMultiLineLint(r""" // /* comment, but not multi-line""", '') def testMultilineStrings(self): multiline_string_error_message = ( @@ -809,131 +849,151 @@ class CpplintTest(CpplintTestBase): def testExplicitSingleArgumentConstructors(self): # missing explicit is bad self.TestMultiLineLint( - '''class Foo { + """class Foo { Foo(int f); - };''', + };""", 'Single-argument constructors should be marked explicit.' ' [runtime/explicit] [5]') # missing explicit is bad, even with whitespace self.TestMultiLineLint( - '''class Foo { + """class Foo { Foo (int f); - };''', + };""", ['Extra space before ( in function call [whitespace/parens] [4]', 'Single-argument constructors should be marked explicit.' ' [runtime/explicit] [5]']) # missing explicit, with distracting comment, is still bad self.TestMultiLineLint( - '''class Foo { + """class Foo { Foo(int f); // simpler than Foo(blargh, blarg) - };''', + };""", 'Single-argument constructors should be marked explicit.' ' [runtime/explicit] [5]') # missing explicit, with qualified classname self.TestMultiLineLint( - '''class Qualifier::AnotherOne::Foo { + """class Qualifier::AnotherOne::Foo { Foo(int f); - };''', + };""", + 'Single-argument constructors should be marked explicit.' + ' [runtime/explicit] [5]') + # missing explicit for inline constructors is bad as well + self.TestMultiLineLint( + """class Foo { + inline Foo(int f); + };""", 'Single-argument constructors should be marked explicit.' ' [runtime/explicit] [5]') # structs are caught as well. self.TestMultiLineLint( - '''struct Foo { + """struct Foo { Foo(int f); - };''', + };""", 'Single-argument constructors should be marked explicit.' ' [runtime/explicit] [5]') # Templatized classes are caught as well. self.TestMultiLineLint( - '''template class Foo { + """template class Foo { Foo(int f); - };''', + };""", + 'Single-argument constructors should be marked explicit.' + ' [runtime/explicit] [5]') + # inline case for templatized classes. + self.TestMultiLineLint( + """template class Foo { + inline Foo(int f); + };""", 'Single-argument constructors should be marked explicit.' ' [runtime/explicit] [5]') # proper style is okay self.TestMultiLineLint( - '''class Foo { + """class Foo { explicit Foo(int f); - };''', + };""", '') # two argument constructor is okay self.TestMultiLineLint( - '''class Foo { + """class Foo { Foo(int f, int b); - };''', + };""", '') # two argument constructor, across two lines, is okay self.TestMultiLineLint( - '''class Foo { + """class Foo { Foo(int f, int b); - };''', + };""", '') # non-constructor (but similar name), is okay self.TestMultiLineLint( - '''class Foo { + """class Foo { aFoo(int f); - };''', + };""", '') # constructor with void argument is okay self.TestMultiLineLint( - '''class Foo { + """class Foo { Foo(void); - };''', + };""", '') # single argument method is okay self.TestMultiLineLint( - '''class Foo { + """class Foo { Bar(int b); - };''', + };""", '') # comments should be ignored self.TestMultiLineLint( - '''class Foo { + """class Foo { // Foo(int f); - };''', + };""", '') # single argument function following class definition is okay # (okay, it's not actually valid, but we don't want a false positive) self.TestMultiLineLint( - '''class Foo { + """class Foo { Foo(int f, int b); }; - Foo(int f);''', + Foo(int f);""", '') # single argument function is okay self.TestMultiLineLint( - '''static Foo(int f);''', + """static Foo(int f);""", '') # single argument copy constructor is okay. self.TestMultiLineLint( - '''class Foo { + """class Foo { Foo(const Foo&); - };''', + };""", '') self.TestMultiLineLint( - '''class Foo { + """class Foo { Foo(Foo&); - };''', + };""", + '') + # templatized copy constructor is okay. + self.TestMultiLineLint( + """template class Foo { + Foo(const Foo&); + };""", '') def testSlashStarCommentOnSingleLine(self): self.TestMultiLineLint( - '''/* static */ Foo(int f);''', + """/* static */ Foo(int f);""", '') self.TestMultiLineLint( - '''/*/ static */ Foo(int f);''', + """/*/ static */ Foo(int f);""", '') self.TestMultiLineLint( - '''/*/ static Foo(int f);''', + """/*/ static Foo(int f);""", 'Could not find end of multi-line comment' ' [readability/multiline_comment] [5]') self.TestMultiLineLint( - ''' /*/ static Foo(int f);''', + """ /*/ static Foo(int f);""", 'Could not find end of multi-line comment' ' [readability/multiline_comment] [5]') self.TestMultiLineLint( - ''' /**/ static Foo(int f);''', + """ /**/ static Foo(int f);""", '') # Test suspicious usage of "if" like this: @@ -1017,6 +1077,9 @@ class CpplintTest(CpplintTestBase): self.TestLint('printf("foo")', '') self.TestLint('printf("foo: %s", foo)', '') self.TestLint('DocidForPrintf(docid)', '') # Should not trigger. + self.TestLint('printf(format, value)', '') # Should not trigger. + self.TestLint('printf(format.c_str(), value)', '') # Should not trigger. + self.TestLint('printf(format(index).c_str(), value)', '') self.TestLint( 'printf(foo)', 'Potential format string bug. Do printf("%s", foo) instead.' @@ -1121,15 +1184,27 @@ class CpplintTest(CpplintTestBase): 'DISALLOW_IMPLICIT_CONSTRUCTORS'): self.TestLanguageRulesCheck( 'some_class.h', - '''%s(SomeClass); + """%s(SomeClass); int foo_; - };''' % macro_name, + };""" % macro_name, ('%s should be the last thing in the class' % macro_name) + ' [readability/constructors] [3]') self.TestLanguageRulesCheck( 'some_class.h', - '''%s(SomeClass); - };''' % macro_name, + """%s(SomeClass); + };""" % macro_name, + '') + self.TestLanguageRulesCheck( + 'some_class.h', + """%s(SomeClass); + int foo_; + } instance, *pointer_to_instance;""" % macro_name, + ('%s should be the last thing in the class' % macro_name) + + ' [readability/constructors] [3]') + self.TestLanguageRulesCheck( + 'some_class.h', + """%s(SomeClass); + } instance, *pointer_to_instance;""" % macro_name, '') # Brace usage @@ -1138,23 +1213,23 @@ class CpplintTest(CpplintTestBase): # or initializing an array self.TestLint('int a[3] = { 1, 2, 3 };', '') self.TestLint( - '''const int foo[] = - {1, 2, 3 };''', + """const int foo[] = + {1, 2, 3 };""", '') # For single line, unmatched '}' with a ';' is ignored (not enough context) self.TestMultiLineLint( - '''int a[3] = { 1, + """int a[3] = { 1, 2, - 3 };''', + 3 };""", '') self.TestMultiLineLint( - '''int a[2][3] = { { 1, 2 }, - { 3, 4 } };''', + """int a[2][3] = { { 1, 2 }, + { 3, 4 } };""", '') self.TestMultiLineLint( - '''int a[2][3] = + """int a[2][3] = { { 1, 2 }, - { 3, 4 } };''', + { 3, 4 } };""", '') # CHECK/EXPECT_TRUE/EXPECT_FALSE replacements @@ -1394,6 +1469,10 @@ class CpplintTest(CpplintTestBase): self.TestLint('} else {', '') self.TestLint('} else if', '') + def testSpacingWithInitializerLists(self): + self.TestLint('int v[1][3] = {{1, 2, 3}};', '') + self.TestLint('int v[1][1] = {{0}};', '') + def testSpacingForBinaryOps(self): self.TestLint('if (foo<=bar) {', 'Missing spaces around <=' ' [whitespace/operators] [3]') @@ -1483,6 +1562,19 @@ class CpplintTest(CpplintTestBase): ' [whitespace/parens] [2]') self.TestLint('TellStory(1 /* wolf */, 3 /* pigs */);', '') + self.TestMultiLineLint("""TellStory(1, 3 + );""", + 'Closing ) should be moved to the previous line' + ' [whitespace/parens] [2]') + self.TestMultiLineLint("""TellStory(Wolves(1), + Pigs(3 + ));""", + 'Closing ) should be moved to the previous line' + ' [whitespace/parens] [2]') + self.TestMultiLineLint("""TellStory(1, + 3 );""", + 'Extra space before )' + ' [whitespace/parens] [2]') def testToDoComments(self): start_space = ('Too many spaces before TODO' @@ -1648,6 +1740,59 @@ class CpplintTest(CpplintTestBase): 'Blank line at the end of a code block. Is this needed?' ' [whitespace/blank_line] [3]')) + def testBlankLineBeforeSectionKeyword(self): + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('foo.cc', 'cc', + ['class A {', + ' public:', + ' protected:', # warning 1 + ' private:', # warning 2 + ' struct B {', + ' public:', + ' private:'] + # warning 3 + ([''] * 100) + # Make A and B longer than 100 lines + [' };', + ' struct C {', + ' protected:', + ' private:', # C is too short for warnings + ' };', + '};', + 'class D', + ' : public {', + ' public:', # no warning + '};'], + error_collector) + self.assertEquals(2, error_collector.Results().count( + '"private:" should be preceded by a blank line' + ' [whitespace/blank_line] [3]')) + self.assertEquals(1, error_collector.Results().count( + '"protected:" should be preceded by a blank line' + ' [whitespace/blank_line] [3]')) + + def testNoBlankLineAfterSectionKeyword(self): + error_collector = ErrorCollector(self.assert_) + cpplint.ProcessFileData('foo.cc', 'cc', + ['class A {', + ' public:', + '', # warning 1 + ' private:', + '', # warning 2 + ' struct B {', + ' protected:', + '', # warning 3 + ' };', + '};'], + error_collector) + self.assertEquals(1, error_collector.Results().count( + 'Do not leave a blank line after "public:"' + ' [whitespace/blank_line] [3]')) + self.assertEquals(1, error_collector.Results().count( + 'Do not leave a blank line after "protected:"' + ' [whitespace/blank_line] [3]')) + self.assertEquals(1, error_collector.Results().count( + 'Do not leave a blank line after "private:"' + ' [whitespace/blank_line] [3]')) + def testElseOnSameLineAsClosingBraces(self): error_collector = ErrorCollector(self.assert_) cpplint.ProcessFileData('foo.cc', 'cc', @@ -1796,7 +1941,7 @@ class CpplintTest(CpplintTestBase): def testDefaultFilter(self): default_filters = cpplint._DEFAULT_FILTERS old_filters = cpplint._cpplint_state.filters - cpplint._DEFAULT_FILTERS = [ '-whitespace' ] + cpplint._DEFAULT_FILTERS = ['-whitespace'] try: # Reset filters cpplint._cpplint_state.SetFilters('') @@ -1838,19 +1983,19 @@ class CpplintTest(CpplintTestBase): 'class Foo;', '') self.TestMultiLineLint( - '''struct Foo* - foo = NewFoo();''', + """struct Foo* + foo = NewFoo();""", '') # Here is an example where the linter gets confused, even though # the code doesn't violate the style guide. self.TestMultiLineLint( - '''class Foo + """class Foo #ifdef DERIVE_FROM_GOO : public Goo { #else : public Hoo { #endif - };''', + };""", 'Failed to find complete declaration of class Foo' ' [build/class] [5]') @@ -2117,7 +2262,7 @@ class CpplintTest(CpplintTestBase): ' [runtime/printf_format] [3]') self.TestLint( - r'snprintf(file, "Never mix %d and %1$d parmaeters!", value);', + r'snprintf(file, "Never mix %d and %1$d parameters!", value);', '%N$ formats are unconventional. Try rewriting to avoid them.' ' [runtime/printf_format] [2]') @@ -2774,48 +2919,48 @@ class NoNonVirtualDestructorsTest(CpplintTestBase): def testNoError(self): self.TestMultiLineLint( - '''class Foo { + """class Foo { virtual ~Foo(); virtual void foo(); - };''', + };""", '') self.TestMultiLineLint( - '''class Foo { + """class Foo { virtual inline ~Foo(); virtual void foo(); - };''', + };""", '') self.TestMultiLineLint( - '''class Foo { + """class Foo { inline virtual ~Foo(); virtual void foo(); - };''', + };""", '') self.TestMultiLineLint( - '''class Foo::Goo { + """class Foo::Goo { virtual ~Goo(); virtual void goo(); - };''', + };""", '') self.TestMultiLineLint( 'class Foo { void foo(); };', 'More than one command on the same line [whitespace/newline] [4]') self.TestMultiLineLint( - '''class Qualified::Goo : public Foo { + """class Qualified::Goo : public Foo { virtual void goo(); - };''', + };""", '') self.TestMultiLineLint( # Line-ending : - '''class Goo : + """class Goo : public Foo { virtual void goo(); - };''', + };""", 'Labels should always be indented at least one space. ' 'If this is a member-initializer list in a constructor or ' 'the base class list in a class definition, the colon should ' @@ -2823,97 +2968,97 @@ class NoNonVirtualDestructorsTest(CpplintTestBase): def testNoDestructorWhenVirtualNeeded(self): self.TestMultiLineLintRE( - '''class Foo { + """class Foo { virtual void foo(); - };''', + };""", 'The class Foo probably needs a virtual destructor') def testDestructorNonVirtualWhenVirtualNeeded(self): self.TestMultiLineLintRE( - '''class Foo { + """class Foo { ~Foo(); virtual void foo(); - };''', + };""", 'The class Foo probably needs a virtual destructor') def testNoWarnWhenDerived(self): self.TestMultiLineLint( - '''class Foo : public Goo { + """class Foo : public Goo { virtual void foo(); - };''', + };""", '') def testNoDestructorWhenVirtualNeededClassDecorated(self): self.TestMultiLineLintRE( - '''class LOCKABLE API Foo { + """class LOCKABLE API Foo { virtual void foo(); - };''', + };""", 'The class Foo probably needs a virtual destructor') def testDestructorNonVirtualWhenVirtualNeededClassDecorated(self): self.TestMultiLineLintRE( - '''class LOCKABLE API Foo { + """class LOCKABLE API Foo { ~Foo(); virtual void foo(); - };''', + };""", 'The class Foo probably needs a virtual destructor') def testNoWarnWhenDerivedClassDecorated(self): self.TestMultiLineLint( - '''class LOCKABLE API Foo : public Goo { + """class LOCKABLE API Foo : public Goo { virtual void foo(); - };''', + };""", '') def testInternalBraces(self): self.TestMultiLineLintRE( - '''class Foo { + """class Foo { enum Goo { GOO }; virtual void foo(); - };''', + };""", 'The class Foo probably needs a virtual destructor') def testInnerClassNeedsVirtualDestructor(self): self.TestMultiLineLintRE( - '''class Foo { + """class Foo { class Goo { virtual void goo(); }; - };''', + };""", 'The class Goo probably needs a virtual destructor') def testOuterClassNeedsVirtualDestructor(self): self.TestMultiLineLintRE( - '''class Foo { + """class Foo { class Goo { }; virtual void foo(); - };''', + };""", 'The class Foo probably needs a virtual destructor') def testQualifiedClassNeedsVirtualDestructor(self): self.TestMultiLineLintRE( - '''class Qualified::Foo { + """class Qualified::Foo { virtual void foo(); - };''', + };""", 'The class Qualified::Foo probably needs a virtual destructor') def testMultiLineDeclarationNoError(self): self.TestMultiLineLintRE( - '''class Foo + """class Foo : public Goo { virtual void foo(); - };''', + };""", '') def testMultiLineDeclarationWithError(self): self.TestMultiLineLint( - '''class Foo + """class Foo { virtual void foo(); - };''', + };""", ['{ should almost always be at the end of the previous line ' '[whitespace/braces] [4]', 'The class Foo probably needs a virtual destructor due to having ' @@ -2925,10 +3070,22 @@ class NoNonVirtualDestructorsTest(CpplintTestBase): 'If you can, use sizeof(fisk) instead of 1 as the 2nd arg ' 'to snprintf. [runtime/printf] [3]') + def testExplicitMakePair(self): + self.TestLint('make_pair', '') + self.TestLint('make_pair(42, 42)', '') + self.TestLint('make_pair<', + 'Omit template arguments from make_pair OR use pair directly' + ' OR if appropriate, construct a pair directly' + ' [build/explicit_make_pair] [4]') + self.TestLint('make_pair <', + 'Omit template arguments from make_pair OR use pair directly' + ' OR if appropriate, construct a pair directly' + ' [build/explicit_make_pair] [4]') + self.TestLint('my_make_pair', '') # pylint: disable-msg=C6409 def setUp(): - """ Runs before all tests are executed. + """Runs before all tests are executed. """ # Enable all filters, so we don't miss anything that is off by default. cpplint._DEFAULT_FILTERS = []