mirror of
https://github.com/google/styleguide.git
synced 2024-03-22 13:11:43 +08:00
Update cpplint.py to #161:
- ostream should be treated as a system header
- Expand the MockCallback whitelist entry for gMock.
- Virtual destructor check shouldn't get confused with
"class EXPORT ClassName {" constructs.
- Don't warn about the length of "$ Id: ... $" lines.
- Better semicolon checks in for() loops.
- Better whitespace comment checks.
Review URL: http://codereview.appspot.com/4964064
This commit is contained in:
erg@google.com
parent
8f91ab2835
commit
d7d2747c6c
48
cpplint/cpplint.py
vendored
48
cpplint/cpplint.py
vendored
|
|
@ -235,11 +235,11 @@ _CPP_HEADERS = frozenset([
|
|||
'cstdio', 'cstdlib', 'cstring', 'ctime', 'cwchar', 'cwctype',
|
||||
'defalloc.h', 'deque.h', 'editbuf.h', 'exception', 'fstream',
|
||||
'fstream.h', 'hashtable.h', 'heap.h', 'indstream.h', 'iomanip',
|
||||
'iomanip.h', 'ios', 'iosfwd', 'iostream', 'iostream.h', 'istream.h',
|
||||
'iterator.h', 'limits', 'map.h', 'multimap.h', 'multiset.h',
|
||||
'numeric', 'ostream.h', 'parsestream.h', 'pfstream.h', 'PlotFile.h',
|
||||
'procbuf.h', 'pthread_alloc.h', 'rope', 'rope.h', 'ropeimpl.h',
|
||||
'SFile.h', 'slist', 'slist.h', 'stack.h', 'stdexcept',
|
||||
'iomanip.h', 'ios', 'iosfwd', 'iostream', 'iostream.h', 'istream',
|
||||
'istream.h', 'iterator.h', 'limits', 'map.h', 'multimap.h', 'multiset.h',
|
||||
'numeric', 'ostream', 'ostream.h', 'parsestream.h', 'pfstream.h',
|
||||
'PlotFile.h', 'procbuf.h', 'pthread_alloc.h', 'rope', 'rope.h',
|
||||
'ropeimpl.h', 'SFile.h', 'slist', 'slist.h', 'stack.h', 'stdexcept',
|
||||
'stdiostream.h', 'streambuf.h', 'stream.h', 'strfile.h', 'string',
|
||||
'strstream', 'strstream.h', 'tempbuf.h', 'tree.h', 'typeinfo', 'valarray',
|
||||
])
|
||||
|
|
@ -913,7 +913,7 @@ def CleanseComments(line):
|
|||
"""
|
||||
commentpos = line.find('//')
|
||||
if commentpos != -1 and not IsCppString(line[:commentpos]):
|
||||
line = line[:commentpos]
|
||||
line = line[:commentpos].rstrip()
|
||||
# get rid of /* ... */
|
||||
return _RE_PATTERN_CLEANSE_LINE_C_COMMENTS.sub('', line)
|
||||
|
||||
|
|
@ -1378,11 +1378,14 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum,
|
|||
# style guidelines, but it seems to perform well enough in testing
|
||||
# to be a worthwhile addition to the checks.
|
||||
classinfo_stack = class_state.classinfo_stack
|
||||
# Look for a class declaration
|
||||
# Look for a class declaration. The regexp accounts for decorated classes
|
||||
# such as in:
|
||||
# class LOCKABLE API Object {
|
||||
# };
|
||||
class_decl_match = Match(
|
||||
r'\s*(template\s*<[\w\s<>,:]*>\s*)?(class|struct)\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(3), linenum))
|
||||
classinfo_stack.append(_ClassInfo(class_decl_match.group(4), linenum))
|
||||
|
||||
# Everything else in this function uses the top of the stack if it's
|
||||
# not empty.
|
||||
|
|
@ -1824,6 +1827,14 @@ def CheckSpacing(filename, clean_lines, linenum, error):
|
|||
error(filename, linenum, 'whitespace/comma', 3,
|
||||
'Missing space after ,')
|
||||
|
||||
# You should always have a space after a semicolon
|
||||
# except for few corner cases
|
||||
# TODO(unknown): clarify if 'if (1) { return 1;}' is requires one more
|
||||
# space after ;
|
||||
if Search(r';[^\s};\\)/]', line):
|
||||
error(filename, linenum, 'whitespace/semicolon', 3,
|
||||
'Missing space after ;')
|
||||
|
||||
# Next we will look for issues with function calls.
|
||||
CheckSpacingForFunctionCall(filename, line, linenum, error)
|
||||
|
||||
|
|
@ -2119,8 +2130,12 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, error):
|
|||
#
|
||||
# URLs can be long too. It's possible to split these, but it makes them
|
||||
# harder to cut&paste.
|
||||
#
|
||||
# The "$Id:...$" comment may also get very long without it being the
|
||||
# developers fault.
|
||||
if (not line.startswith('#include') and not is_header_guard and
|
||||
not Match(r'^\s*//.*http(s?)://\S*$', line)):
|
||||
not Match(r'^\s*//.*http(s?)://\S*$', line) and
|
||||
not Match(r'^// \$Id:.*#[0-9]+ \$$', line)):
|
||||
line_width = GetLineWidth(line)
|
||||
if line_width > 100:
|
||||
error(filename, linenum, 'whitespace/line_length', 4,
|
||||
|
|
@ -2402,9 +2417,12 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state,
|
|||
if match:
|
||||
# gMock methods are defined using some variant of MOCK_METHODx(name, type)
|
||||
# where type may be float(), int(string), etc. Without context they are
|
||||
# virtually indistinguishable from int(x) casts.
|
||||
# virtually indistinguishable from int(x) casts. Likewise, gMock's
|
||||
# MockCallback takes a template parameter of the form return_type(arg_type),
|
||||
# which looks much like the cast we're trying to detect.
|
||||
if (match.group(1) is None and # If new operator, then this isn't a cast
|
||||
not Match(r'^\s*MOCK_(CONST_)?METHOD\d+(_T)?\(', line)):
|
||||
not (Match(r'^\s*MOCK_(CONST_)?METHOD\d+(_T)?\(', line) or
|
||||
Match(r'^\s*MockCallback<.*>', line))):
|
||||
error(filename, linenum, 'readability/casting', 4,
|
||||
'Using deprecated casting style. '
|
||||
'Use static_cast<%s>(...) instead' %
|
||||
|
|
@ -2634,15 +2652,17 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern,
|
|||
# eg, void foo(int); or void foo(int) const;
|
||||
# The equals check is for function pointer assignment.
|
||||
# eg, void *(*foo)(int) = ...
|
||||
# The > is for MockCallback<...> ...
|
||||
#
|
||||
# Right now, this will only catch cases where there's a single argument, and
|
||||
# it's unnamed. It should probably be expanded to check for multiple
|
||||
# arguments with some unnamed.
|
||||
function_match = Match(r'\s*(\)|=|(const)?\s*(;|\{|throw\(\)))', remainder)
|
||||
function_match = Match(r'\s*(\)|=|(const)?\s*(;|\{|throw\(\)|>))', remainder)
|
||||
if function_match:
|
||||
if (not function_match.group(3) or
|
||||
function_match.group(3) == ';' or
|
||||
raw_line.find('/*') < 0):
|
||||
('MockCallback<' not in raw_line and
|
||||
'/*' not in raw_line)):
|
||||
error(filename, linenum, 'readability/function', 3,
|
||||
'All parameters should be named in a function')
|
||||
return
|
||||
|
|
|
|||
|
|
@ -294,6 +294,13 @@ class CpplintTest(CpplintTestBase):
|
|||
self.TestLint(
|
||||
'// Read https://g' + ('o' * 60) + 'gle.com/' ,
|
||||
'')
|
||||
self.TestLint(
|
||||
'// $Id: g' + ('o' * 80) + 'gle.cc#1 $',
|
||||
'')
|
||||
self.TestLint(
|
||||
'// $Id: g' + ('o' * 80) + 'gle.cc#1',
|
||||
'Lines should be <= 80 characters long'
|
||||
' [whitespace/line_length] [2]')
|
||||
|
||||
# Test error suppression annotations.
|
||||
def testErrorSuppression(self):
|
||||
|
|
@ -490,6 +497,16 @@ class CpplintTest(CpplintTestBase):
|
|||
'MOCK_CONST_METHOD2_T(method, double(float, float));',
|
||||
'')
|
||||
|
||||
# Like gMock method definitions, MockCallback instantiations look very similar
|
||||
# to bad casts.
|
||||
def testMockCallback(self):
|
||||
self.TestLint(
|
||||
'MockCallback<bool(int)>',
|
||||
'')
|
||||
self.TestLint(
|
||||
'MockCallback<int(float, char)>',
|
||||
'')
|
||||
|
||||
# Test sizeof(type) cases.
|
||||
def testSizeofType(self):
|
||||
self.TestLint(
|
||||
|
|
@ -737,7 +754,7 @@ class CpplintTest(CpplintTestBase):
|
|||
self.assertEquals((False, ''), f('a.cc', 'b.h'))
|
||||
|
||||
def testCleanseLine(self):
|
||||
self.assertEquals('int foo = 0; ',
|
||||
self.assertEquals('int foo = 0;',
|
||||
cpplint.CleanseComments('int foo = 0; // danger!'))
|
||||
self.assertEquals('int o = 0;',
|
||||
cpplint.CleanseComments('int /* foo */ o = 0;'))
|
||||
|
|
@ -1288,7 +1305,7 @@ class CpplintTest(CpplintTestBase):
|
|||
self.TestLint('void foo(const typename tm& tm);', '')
|
||||
# Returning an address of something is not prohibited.
|
||||
self.TestLint('return &something;', '')
|
||||
self.TestLint('if (condition) {return &something;}', '')
|
||||
self.TestLint('if (condition) {return &something; }', '')
|
||||
self.TestLint('if (condition) return &something;', '')
|
||||
self.TestLint('if (condition) address = &something;', '')
|
||||
self.TestLint('if (condition) result = lhs&rhs;', '')
|
||||
|
|
@ -1317,11 +1334,29 @@ class CpplintTest(CpplintTestBase):
|
|||
|
||||
def testSpacingForFncall(self):
|
||||
self.TestLint('if (foo) {', '')
|
||||
self.TestLint('for (foo;bar;baz) {', '')
|
||||
self.TestLint('for (foo; bar; baz) {', '')
|
||||
self.TestLint('for (;;) {', '')
|
||||
# Test that there is no warning when increment statement is empty.
|
||||
self.TestLint('for (foo; baz;) {', '')
|
||||
self.TestLint('for (foo;bar;baz) {', 'Missing space after ;'
|
||||
' [whitespace/semicolon] [3]')
|
||||
# we don't warn about this semicolon, at least for now
|
||||
self.TestLint('if (condition) {return &something; }',
|
||||
'')
|
||||
# seen in some macros
|
||||
self.TestLint('DoSth();\\', '')
|
||||
# Test that there is no warning about semicolon here.
|
||||
self.TestLint('abc;// this is abc',
|
||||
'At least two spaces is best between code'
|
||||
' and comments [whitespace/comments] [2]')
|
||||
self.TestLint('while (foo) {', '')
|
||||
self.TestLint('switch (foo) {', '')
|
||||
self.TestLint('foo( bar)', 'Extra space after ( in function call'
|
||||
' [whitespace/parens] [4]')
|
||||
self.TestLint('foo( // comment', '')
|
||||
self.TestLint('foo( // comment',
|
||||
'At least two spaces is best between code'
|
||||
' and comments [whitespace/comments] [2]')
|
||||
self.TestLint('foobar( \\', '')
|
||||
self.TestLint('foobar( \\', '')
|
||||
self.TestLint('( a + b)', 'Extra space after ('
|
||||
|
|
@ -2204,22 +2239,26 @@ class CleansedLinesTest(unittest.TestCase):
|
|||
lines = ['Line 1',
|
||||
'Line 2',
|
||||
'Line 3 // Comment test',
|
||||
'Line 4 "foo"']
|
||||
'Line 4 /* Comment test */',
|
||||
'Line 5 "foo"']
|
||||
|
||||
|
||||
clean_lines = cpplint.CleansedLines(lines)
|
||||
self.assertEquals(lines, clean_lines.raw_lines)
|
||||
self.assertEquals(4, clean_lines.NumLines())
|
||||
self.assertEquals(5, clean_lines.NumLines())
|
||||
|
||||
self.assertEquals(['Line 1',
|
||||
'Line 2',
|
||||
'Line 3 ',
|
||||
'Line 4 "foo"'],
|
||||
'Line 3',
|
||||
'Line 4',
|
||||
'Line 5 "foo"'],
|
||||
clean_lines.lines)
|
||||
|
||||
self.assertEquals(['Line 1',
|
||||
'Line 2',
|
||||
'Line 3 ',
|
||||
'Line 4 ""'],
|
||||
'Line 3',
|
||||
'Line 4',
|
||||
'Line 5 ""'],
|
||||
clean_lines.elided)
|
||||
|
||||
def testInitEmpty(self):
|
||||
|
|
@ -2804,6 +2843,28 @@ class NoNonVirtualDestructorsTest(CpplintTestBase):
|
|||
};''',
|
||||
'')
|
||||
|
||||
def testNoDestructorWhenVirtualNeededClassDecorated(self):
|
||||
self.TestMultiLineLintRE(
|
||||
'''class LOCKABLE API Foo {
|
||||
virtual void foo();
|
||||
};''',
|
||||
'The class Foo probably needs a virtual destructor')
|
||||
|
||||
def testDestructorNonVirtualWhenVirtualNeededClassDecorated(self):
|
||||
self.TestMultiLineLintRE(
|
||||
'''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 {
|
||||
virtual void foo();
|
||||
};''',
|
||||
'')
|
||||
|
||||
def testInternalBraces(self):
|
||||
self.TestMultiLineLintRE(
|
||||
'''class Foo {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user