C++ style checker should warn when the indentation is wrong
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Sep 2012 20:06:01 +0000 (20:06 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Sep 2012 20:06:01 +0000 (20:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97602

Reviewed by Ojan Vafai.

Rewrite the indentation checker to ensure that indentation is always a factor of 4
and that we only indent 4 spaces from the previous line.

I deleted some of the old indentation checking code since it was filtered out
and didn't match the webkit style.

* Scripts/webkitpy/style/checker.py:
* Scripts/webkitpy/style/checkers/cpp.py:
(check_indentation_amount): New function that checks indentation. Intentionally runs after
other indentation checks.
(check_style): Delete the old indentation code since we were already filtering out the whitespace/labels warnings.
(CppChecker): Remove the whitespace/labels filter.
* Scripts/webkitpy/style/checkers/cpp_unittest.py:
(CppStyleTest.test_multi_line_comments): Force 4 space indent.
(CppStyleTest.test_explicit_single_argument_constructors):
(CppStyleTest.test_explicit_single_argument_constructors.Foo):
(CppStyleTest.test_explicit_single_argument_constructors.Qualifier):
(CppStyleTest.test_slash_star_comment_on_single_line):
(CppStyleTest.test_braces):
(CppStyleTest.test_spacing_before_last_semicolon):
(CppStyleTest.test_static_or_global_stlstrings):
(CppStyleTest.test_indent): Added macro test case.
(CppStyleTest.test_build_class):
(NoNonVirtualDestructorsTest.test_no_error):
(NoNonVirtualDestructorsTest.test_no_error.Foo):
(NoNonVirtualDestructorsTest.test_no_error.Qualified):
(NoNonVirtualDestructorsTest.test_no_destructor_when_virtual_needed):
(NoNonVirtualDestructorsTest.test_no_destructor_when_virtual_needed.Foo):
(NoNonVirtualDestructorsTest.test_destructor_non_virtual_when_virtual_needed):
(NoNonVirtualDestructorsTest.test_destructor_non_virtual_when_virtual_needed.Foo):
(NoNonVirtualDestructorsTest.test_no_warn_when_derived):
(NoNonVirtualDestructorsTest.test_no_warn_when_derived.Foo):
(NoNonVirtualDestructorsTest.test_internal_braces):
(NoNonVirtualDestructorsTest.test_internal_braces.Foo):
(NoNonVirtualDestructorsTest.test_inner_class_needs_virtual_destructor):
(NoNonVirtualDestructorsTest.test_inner_class_needs_virtual_destructor.Foo):
(NoNonVirtualDestructorsTest.test_inner_class_needs_virtual_destructor.Foo.Goo):
(NoNonVirtualDestructorsTest.test_outer_class_needs_virtual_destructor):
(NoNonVirtualDestructorsTest.test_outer_class_needs_virtual_destructor.Foo):
(NoNonVirtualDestructorsTest.test_outer_class_needs_virtual_destructor.Foo.Goo):
(NoNonVirtualDestructorsTest.test_qualified_class_needs_virtual_destructor):
(NoNonVirtualDestructorsTest.test_qualified_class_needs_virtual_destructor.Qualified):
(NoNonVirtualDestructorsTest.test_multi_line_declaration_no_error):
(NoNonVirtualDestructorsTest.test_multi_line_declaration_no_error.Foo):
(NoNonVirtualDestructorsTest.test_multi_line_declaration_with_error):
(NoNonVirtualDestructorsTest.test_multi_line_declaration_with_error.Foo):
(WebKitStyleTest.test_indentation):
(WebKitStyleTest.test_braces): Add test for call params spanning lines.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@129690 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/style/checker.py
Tools/Scripts/webkitpy/style/checkers/cpp.py
Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py

index 01cf0e3..d27b00f 100644 (file)
@@ -1,3 +1,59 @@
+2012-09-26  Tony Chang  <tony@chromium.org>
+
+        C++ style checker should warn when the indentation is wrong
+        https://bugs.webkit.org/show_bug.cgi?id=97602
+
+        Reviewed by Ojan Vafai.
+
+        Rewrite the indentation checker to ensure that indentation is always a factor of 4
+        and that we only indent 4 spaces from the previous line.
+
+        I deleted some of the old indentation checking code since it was filtered out
+        and didn't match the webkit style.
+
+        * Scripts/webkitpy/style/checker.py:
+        * Scripts/webkitpy/style/checkers/cpp.py:
+        (check_indentation_amount): New function that checks indentation. Intentionally runs after
+        other indentation checks.
+        (check_style): Delete the old indentation code since we were already filtering out the whitespace/labels warnings.
+        (CppChecker): Remove the whitespace/labels filter.
+        * Scripts/webkitpy/style/checkers/cpp_unittest.py:
+        (CppStyleTest.test_multi_line_comments): Force 4 space indent.
+        (CppStyleTest.test_explicit_single_argument_constructors):
+        (CppStyleTest.test_explicit_single_argument_constructors.Foo):
+        (CppStyleTest.test_explicit_single_argument_constructors.Qualifier):
+        (CppStyleTest.test_slash_star_comment_on_single_line):
+        (CppStyleTest.test_braces):
+        (CppStyleTest.test_spacing_before_last_semicolon):
+        (CppStyleTest.test_static_or_global_stlstrings):
+        (CppStyleTest.test_indent): Added macro test case.
+        (CppStyleTest.test_build_class):
+        (NoNonVirtualDestructorsTest.test_no_error):
+        (NoNonVirtualDestructorsTest.test_no_error.Foo):
+        (NoNonVirtualDestructorsTest.test_no_error.Qualified):
+        (NoNonVirtualDestructorsTest.test_no_destructor_when_virtual_needed):
+        (NoNonVirtualDestructorsTest.test_no_destructor_when_virtual_needed.Foo):
+        (NoNonVirtualDestructorsTest.test_destructor_non_virtual_when_virtual_needed):
+        (NoNonVirtualDestructorsTest.test_destructor_non_virtual_when_virtual_needed.Foo):
+        (NoNonVirtualDestructorsTest.test_no_warn_when_derived):
+        (NoNonVirtualDestructorsTest.test_no_warn_when_derived.Foo):
+        (NoNonVirtualDestructorsTest.test_internal_braces):
+        (NoNonVirtualDestructorsTest.test_internal_braces.Foo):
+        (NoNonVirtualDestructorsTest.test_inner_class_needs_virtual_destructor):
+        (NoNonVirtualDestructorsTest.test_inner_class_needs_virtual_destructor.Foo):
+        (NoNonVirtualDestructorsTest.test_inner_class_needs_virtual_destructor.Foo.Goo):
+        (NoNonVirtualDestructorsTest.test_outer_class_needs_virtual_destructor):
+        (NoNonVirtualDestructorsTest.test_outer_class_needs_virtual_destructor.Foo):
+        (NoNonVirtualDestructorsTest.test_outer_class_needs_virtual_destructor.Foo.Goo):
+        (NoNonVirtualDestructorsTest.test_qualified_class_needs_virtual_destructor):
+        (NoNonVirtualDestructorsTest.test_qualified_class_needs_virtual_destructor.Qualified):
+        (NoNonVirtualDestructorsTest.test_multi_line_declaration_no_error):
+        (NoNonVirtualDestructorsTest.test_multi_line_declaration_no_error.Foo):
+        (NoNonVirtualDestructorsTest.test_multi_line_declaration_with_error):
+        (NoNonVirtualDestructorsTest.test_multi_line_declaration_with_error.Foo):
+        (WebKitStyleTest.test_indentation):
+        (WebKitStyleTest.test_braces): Add test for call params spanning lines.
+
 2012-09-26  Adam Barth  <abarth@webkit.org>
 
         Rollout messages should be clickable in bugzilla
index 5b11af4..9f27c36 100644 (file)
@@ -96,7 +96,6 @@ _BASE_FILTER_RULES = [
     '-runtime/rtti',
     '-whitespace/blank_line',
     '-whitespace/end_of_line',
-    '-whitespace/labels',
     # List Python pep8 categories last.
     #
     # Because much of WebKit's Python code base does not abide by the
index a0051c9..ebbd1ad 100644 (file)
@@ -2060,6 +2060,31 @@ def check_directive_indentation(clean_lines, line_number, file_state, error):
     error(line_number, 'whitespace/indent', 4, 'preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.')
 
 
+def get_initial_spaces_for_line(clean_line):
+    initial_spaces = 0
+    while initial_spaces < len(clean_line) and clean_line[initial_spaces] == ' ':
+        initial_spaces += 1
+    return initial_spaces
+
+
+def check_indentation_amount(clean_lines, line_number, error):
+    line = clean_lines.elided[line_number]
+    initial_spaces = get_initial_spaces_for_line(line)
+
+    if initial_spaces % 4:
+        error(line_number, 'whitespace/indent', 3,
+              'Weird number of spaces at line-start.  Are you using a 4-space indent?')
+        return
+
+    previous_line = get_previous_non_blank_line(clean_lines, line_number)[0]
+    if not previous_line.strip() or match(r'\s*\w+\s*:\s*$', previous_line) or previous_line[0] == '#':
+        return
+
+    previous_line_initial_spaces = get_initial_spaces_for_line(previous_line)
+    if initial_spaces > previous_line_initial_spaces + 4:
+        error(line_number, 'whitespace/indent', 3, 'When wrapping a line, only indent 4 spaces.')
+
+
 def check_using_std(clean_lines, line_number, file_state, error):
     """Looks for 'using std::foo;' statements which should be replaced with 'using namespace std;'.
 
@@ -2535,44 +2560,10 @@ def check_style(clean_lines, line_number, file_extension, class_state, file_stat
         error(line_number, 'whitespace/tab', 1,
               'Tab found; better to use spaces')
 
-    # One or three blank spaces at the beginning of the line is weird; it's
-    # hard to reconcile that with 4-space indents.
-    # NOTE: here are the conditions rob pike used for his tests.  Mine aren't
-    # as sophisticated, but it may be worth becoming so:  RLENGTH==initial_spaces
-    # if(RLENGTH > 20) complain = 0;
-    # if(match($0, " +(error|private|public|protected):")) complain = 0;
-    # if(match(prev, "&& *$")) complain = 0;
-    # if(match(prev, "\\|\\| *$")) complain = 0;
-    # if(match(prev, "[\",=><] *$")) complain = 0;
-    # if(match($0, " <<")) complain = 0;
-    # if(match(prev, " +for \\(")) complain = 0;
-    # if(prevodd && match(prevprev, " +for \\(")) complain = 0;
-    initial_spaces = 0
     cleansed_line = clean_lines.elided[line_number]
-    while initial_spaces < len(line) and line[initial_spaces] == ' ':
-        initial_spaces += 1
     if line and line[-1].isspace():
         error(line_number, 'whitespace/end_of_line', 4,
               'Line ends in whitespace.  Consider deleting these extra spaces.')
-    # There are certain situations we allow one space, notably for labels
-    elif ((initial_spaces >= 1 and initial_spaces <= 3)
-          and not match(r'\s*\w+\s*:\s*$', cleansed_line)):
-        error(line_number, 'whitespace/indent', 3,
-              'Weird number of spaces at line-start.  '
-              'Are you using a 4-space indent?')
-    # Labels should always be indented at least one space.
-    elif not initial_spaces and line[:2] != '//':
-        label_match = match(r'(?P<label>[^:]+):\s*$', line)
-
-        if label_match:
-            label = label_match.group('label')
-            # Only throw errors for stuff that is definitely not a goto label,
-            # because goto labels can in fact occur at the start of the line.
-            if label in ['public', 'private', 'protected'] or label.find(' ') != -1:
-                error(line_number, 'whitespace/labels', 4,
-                      'Labels should always be indented at least one space.  '
-                      'If this is a member-initializer list in a constructor, '
-                      'the colon should be on the line after the definition header.')
 
     if (cleansed_line.count(';') > 1
         # for loops are allowed two ;'s (and may run over two lines).
@@ -2612,6 +2603,7 @@ def check_style(clean_lines, line_number, file_extension, class_state, file_stat
     check_check(clean_lines, line_number, error)
     check_for_comparisons_to_zero(clean_lines, line_number, error)
     check_for_null(clean_lines, line_number, file_state, error)
+    check_indentation_amount(clean_lines, line_number, error)
 
 
 _RE_PATTERN_INCLUDE_NEW_STYLE = re.compile(r'#include +"[^/]+\.h"')
@@ -3634,7 +3626,6 @@ class CppChecker(object):
         'whitespace/end_of_line',
         'whitespace/ending_newline',
         'whitespace/indent',
-        'whitespace/labels',
         'whitespace/line_length',
         'whitespace/newline',
         'whitespace/operators',
index a1b91f2..6f001e0 100644 (file)
@@ -1086,7 +1086,8 @@ class CppStyleTest(CppStyleTestBase):
             */ ''',
         '')
         self.assert_multi_line_lint(
-            r'''/* int a = 0; multi-liner
+            '''\
+            /* int a = 0; multi-liner
             static const int b = 0;''',
             ['Could not find end of multi-line comment'
              '  [readability/multiline_comment] [5]',
@@ -1125,97 +1126,111 @@ class CppStyleTest(CppStyleTestBase):
     def test_explicit_single_argument_constructors(self):
         # missing explicit is bad
         self.assert_multi_line_lint(
-            '''class Foo {
-                 Foo(int f);
-               };''',
+            '''\
+            class Foo {
+                Foo(int f);
+            };''',
             'Single-argument constructors should be marked explicit.'
             '  [runtime/explicit] [5]')
         # missing explicit is bad, even with whitespace
         self.assert_multi_line_lint(
-            '''class Foo {
-                 Foo (int f);
-               };''',
+            '''\
+            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.assert_multi_line_lint(
-            '''class Foo {
-                 Foo(int f); // simpler than Foo(blargh, blarg)
-               };''',
+            '''\
+            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.assert_multi_line_lint(
-            '''class Qualifier::AnotherOne::Foo {
-                 Foo(int f);
-               };''',
+            '''\
+            class Qualifier::AnotherOne::Foo {
+                Foo(int f);
+            };''',
             'Single-argument constructors should be marked explicit.'
             '  [runtime/explicit] [5]')
         # structs are caught as well.
         self.assert_multi_line_lint(
-            '''struct Foo {
-                 Foo(int f);
-               };''',
+            '''\
+            struct Foo {
+                Foo(int f);
+            };''',
             'Single-argument constructors should be marked explicit.'
             '  [runtime/explicit] [5]')
         # Templatized classes are caught as well.
         self.assert_multi_line_lint(
-            '''template<typename T> class Foo {
-                 Foo(int f);
-               };''',
+            '''\
+            template<typename T> class Foo {
+                Foo(int f);
+            };''',
             'Single-argument constructors should be marked explicit.'
             '  [runtime/explicit] [5]')
         # proper style is okay
         self.assert_multi_line_lint(
-            '''class Foo {
-                 explicit Foo(int f);
-               };''',
+            '''\
+            class Foo {
+                explicit Foo(int f);
+            };''',
             '')
         # two argument constructor is okay
         self.assert_multi_line_lint(
-            '''class Foo {
-                 Foo(int f, int b);
-               };''',
+            '''\
+            class Foo {
+                Foo(int f, int b);
+            };''',
             '')
         # two argument constructor, across two lines, is okay
         self.assert_multi_line_lint(
-            '''class Foo {
-                 Foo(int f,
-                     int b);
-               };''',
+            '''\
+            class Foo {
+                Foo(int f,
+                    int b);
+            };''',
             '')
         # non-constructor (but similar name), is okay
         self.assert_multi_line_lint(
-            '''class Foo {
-                 aFoo(int f);
-               };''',
+            '''\
+            class Foo {
+                aFoo(int f);
+            };''',
             '')
         # constructor with void argument is okay
         self.assert_multi_line_lint(
-            '''class Foo {
-                 Foo(void);
-               };''',
+            '''\
+            class Foo {
+                Foo(void);
+            };''',
             '')
         # single argument method is okay
         self.assert_multi_line_lint(
-            '''class Foo {
-                 Bar(int b);
-               };''',
+            '''\
+            class Foo {
+                Bar(int b);
+            };''',
             '')
         # comments should be ignored
         self.assert_multi_line_lint(
-            '''class Foo {
-               // Foo(int f);
-               };''',
+            '''\
+            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.assert_multi_line_lint(
-            '''class Foo {
-                 Foo(int f, int b);
-               };
-               Foo(int f);''',
+            '''\
+            class Foo {
+                Foo(int f, int b);
+            };
+            Foo(int f);''',
             '')
         # single argument function is okay
         self.assert_multi_line_lint(
@@ -1223,14 +1238,16 @@ class CppStyleTest(CppStyleTestBase):
             '')
         # single argument copy constructor is okay.
         self.assert_multi_line_lint(
-            '''class Foo {
-                 Foo(const Foo&);
-               };''',
+            '''\
+            class Foo {
+                Foo(const Foo&);
+            };''',
             '')
         self.assert_multi_line_lint(
-            '''class Foo {
-                 Foo(Foo&);
-               };''',
+            '''\
+            class Foo {
+                Foo(Foo&);
+            };''',
             '')
 
     def test_slash_star_comment_on_single_line(self):
@@ -1248,9 +1265,6 @@ class CppStyleTest(CppStyleTestBase):
             '''    /*/ static Foo(int f);''',
             'Could not find end of multi-line comment'
             '  [readability/multiline_comment] [5]')
-        self.assert_multi_line_lint(
-            '''    /**/ static Foo(int f);''',
-            '')
 
     # Test suspicious usage of "if" like this:
     # if (a == b) {
@@ -1383,23 +1397,27 @@ class CppStyleTest(CppStyleTestBase):
         # or initializing an array
         self.assert_lint('int a[3] = { 1, 2, 3 };', '')
         self.assert_lint(
-            '''const int foo[] =
-                   {1, 2, 3 };''',
+            '''\
+            const int foo[] =
+                {1, 2, 3 };''',
             '')
         # For single line, unmatched '}' with a ';' is ignored (not enough context)
         self.assert_multi_line_lint(
-            '''int a[3] = { 1,
-                            2,
-                            3 };''',
+            '''\
+            int a[3] = { 1,
+                2,
+                3 };''',
             '')
         self.assert_multi_line_lint(
-            '''int a[2][3] = { { 1, 2 },
-                             { 3, 4 } };''',
+            '''\
+            int a[2][3] = { { 1, 2 },
+                { 3, 4 } };''',
             '')
         self.assert_multi_line_lint(
-            '''int a[2][3] =
-                   { { 1, 2 },
-                     { 3, 4 } };''',
+            '''\
+            int a[2][3] =
+                { { 1, 2 },
+                { 3, 4 } };''',
             '')
 
     # CHECK/EXPECT_TRUE/EXPECT_FALSE replacements
@@ -1753,7 +1771,7 @@ class CppStyleTest(CppStyleTestBase):
         self.assert_lint('default:;',
                          'Semicolon defining empty statement. Use { } instead.'
                          '  [whitespace/semicolon] [5]')
-        self.assert_lint('      ;',
+        self.assert_lint('        ;',
                          'Line contains only semicolon. If this should be an empty '
                          'statement, use { } instead.'
                          '  [whitespace/semicolon] [5]')
@@ -1795,10 +1813,10 @@ class CppStyleTest(CppStyleTestBase):
                          '    VeryLongNameType veryLongNameVariable) { }', '')
         self.assert_lint('template<>\n'
                          'string FunctionTemplateSpecialization<SomeType>(\n'
-                         '      int x) { return ""; }', '')
+                         '    int x) { return ""; }', '')
         self.assert_lint('template<>\n'
                          'string FunctionTemplateSpecialization<vector<A::B>* >(\n'
-                         '      int x) { return ""; }', '')
+                         '    int x) { return ""; }', '')
 
         # should not catch methods of template classes.
         self.assert_lint('string Class<Type>::Method() const\n'
@@ -2028,21 +2046,27 @@ class CppStyleTest(CppStyleTestBase):
         self.assert_lint(' char* oneSpaceIndent = "public:";',
                          'Weird number of spaces at line-start.  '
                          'Are you using a 4-space indent?  [whitespace/indent] [3]')
-        self.assert_lint(' public:', '')
-        self.assert_lint('  public:', '')
-        self.assert_lint('   public:', '')
-
-    def test_label(self):
-        self.assert_lint('public:',
-                         'Labels should always be indented at least one space.  '
-                         'If this is a member-initializer list in a constructor, '
-                         'the colon should be on the line after the definition '
-                         'header.  [whitespace/labels] [4]')
-        self.assert_lint('  public:', '')
-        self.assert_lint('   public:', '')
-        self.assert_lint(' public:', '')
-        self.assert_lint('  public:', '')
-        self.assert_lint('   public:', '')
+        self.assert_lint(' public:',
+                         'Weird number of spaces at line-start.  '
+                         'Are you using a 4-space indent?  [whitespace/indent] [3]')
+        self.assert_lint('  public:',
+                         'Weird number of spaces at line-start.  '
+                         'Are you using a 4-space indent?  [whitespace/indent] [3]')
+        self.assert_lint('   public:',
+                         'Weird number of spaces at line-start.  '
+                         'Are you using a 4-space indent?  [whitespace/indent] [3]')
+        self.assert_multi_line_lint(
+            'class Foo {\n'
+            'public:\n'
+            '    enum Bar {\n'
+            '        Alpha,\n'
+            '        Beta,\n'
+            '#if ENABLED_BETZ\n'
+            '        Charlie,\n'
+            '#endif\n'
+            '    };\n'
+            '};',
+            '')
 
     def test_not_alabel(self):
         self.assert_lint('MyVeryLongNamespace::MyVeryLongClassName::', '')
@@ -2080,8 +2104,9 @@ class CppStyleTest(CppStyleTestBase):
             'class Foo;',
             '')
         self.assert_multi_line_lint(
-            '''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.
@@ -3171,31 +3196,35 @@ class NoNonVirtualDestructorsTest(CppStyleTestBase):
 
     def test_no_error(self):
         self.assert_multi_line_lint(
-            '''class Foo {
-                   virtual ~Foo();
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo {
+                    virtual ~Foo();
+                    virtual void foo();
+                };''',
             '')
 
         self.assert_multi_line_lint(
-            '''class Foo {
-                   virtual inline ~Foo();
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo {
+                    virtual inline ~Foo();
+                    virtual void foo();
+                };''',
             '')
 
         self.assert_multi_line_lint(
-            '''class Foo {
-                   inline virtual ~Foo();
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo {
+                    inline virtual ~Foo();
+                    virtual void foo();
+                };''',
             '')
 
         self.assert_multi_line_lint(
-            '''class Foo::Goo {
-                   virtual ~Goo();
-                   virtual void goo();
-               };''',
+            '''\
+                class Foo::Goo {
+                    virtual ~Goo();
+                    virtual void goo();
+                };''',
             '')
         self.assert_multi_line_lint(
             'class Foo { void foo(); };',
@@ -3215,92 +3244,92 @@ class NoNonVirtualDestructorsTest(CppStyleTestBase):
             'More than one command on the same line  [whitespace/newline] [4]')
 
         self.assert_multi_line_lint(
-            '''class Qualified::Goo : public Foo {
-                   virtual void goo();
-               };''',
-            '')
-
-        self.assert_multi_line_lint(
-            # Line-ending :
-            '''class Goo :
-               public Foo {
+            '''\
+                class Qualified::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, the colon should be on the '
-            'line after the definition header.  [whitespace/labels] [4]')
+                };''',
+            '')
 
     def test_no_destructor_when_virtual_needed(self):
         self.assert_multi_line_lint_re(
-            '''class Foo {
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo {
+                    virtual void foo();
+                };''',
             'The class Foo probably needs a virtual destructor')
 
     def test_destructor_non_virtual_when_virtual_needed(self):
         self.assert_multi_line_lint_re(
-            '''class Foo {
-                   ~Foo();
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo {
+                    ~Foo();
+                    virtual void foo();
+                };''',
             'The class Foo probably needs a virtual destructor')
 
     def test_no_warn_when_derived(self):
         self.assert_multi_line_lint(
-            '''class Foo : public Goo {
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo : public Goo {
+                    virtual void foo();
+                };''',
             '')
 
     def test_internal_braces(self):
         self.assert_multi_line_lint_re(
-            '''class Foo {
-                   enum Goo {
-                       GOO
-                   };
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo {
+                    enum Goo {
+                        GOO
+                    };
+                    virtual void foo();
+                };''',
             'The class Foo probably needs a virtual destructor')
 
     def test_inner_class_needs_virtual_destructor(self):
         self.assert_multi_line_lint_re(
-            '''class Foo {
-                   class Goo {
-                       virtual void goo();
-                   };
-               };''',
+            '''\
+                class Foo {
+                    class Goo {
+                        virtual void goo();
+                    };
+                };''',
             'The class Goo probably needs a virtual destructor')
 
     def test_outer_class_needs_virtual_destructor(self):
         self.assert_multi_line_lint_re(
-            '''class Foo {
-                   class Goo {
-                   };
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo {
+                    class Goo {
+                    };
+                    virtual void foo();
+                };''',
             'The class Foo probably needs a virtual destructor')
 
     def test_qualified_class_needs_virtual_destructor(self):
         self.assert_multi_line_lint_re(
-            '''class Qualified::Foo {
-                   virtual void foo();
-               };''',
+            '''\
+                class Qualified::Foo {
+                    virtual void foo();
+                };''',
             'The class Qualified::Foo probably needs a virtual destructor')
 
     def test_multi_line_declaration_no_error(self):
         self.assert_multi_line_lint_re(
-            '''class Foo
-                   : public Goo {
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo
+                    : public Goo {
+                    virtual void foo();
+                };''',
             '')
 
     def test_multi_line_declaration_with_error(self):
         self.assert_multi_line_lint(
-            '''class Foo
-               {
-                   virtual void foo();
-               };''',
+            '''\
+                class Foo
+                {
+                    virtual void foo();
+                };''',
             ['This { should be at the end of the previous line  '
              '[whitespace/braces] [4]',
              'The class Foo probably needs a virtual destructor due to having '
@@ -3495,7 +3524,6 @@ class WebKitStyleTest(CppStyleTestBase):
             '   int goo;\n'
             '};',
             'Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]')
-        # FIXME: No tests for 8-spaces.
 
         # 3. In a header, code inside a namespace should not be indented.
         self.assert_multi_line_lint(
@@ -4180,7 +4208,14 @@ class WebKitStyleTest(CppStyleTestBase):
             '    myFunction(reallyLongParam1, reallyLongParam2,\n'
             '               reallyLongParam3);\n'
             '}\n',
-            '')
+            'Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]')
+
+        self.assert_multi_line_lint(
+            'if (true) {\n'
+            '    myFunction(reallyLongParam1, reallyLongParam2,\n'
+            '            reallyLongParam3);\n'
+            '}\n',
+            'When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]')
 
         # 4. Control clauses without a body should use empty braces.
         self.assert_multi_line_lint(
@@ -4189,7 +4224,7 @@ class WebKitStyleTest(CppStyleTestBase):
         self.assert_multi_line_lint(
             'for ( ; current;\n'
             '     current = current->next) { }\n',
-            '')
+            'Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]')
         self.assert_multi_line_lint(
             'for ( ; current; current = current->next);\n',
             'Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]')