Changeset 46294 in webkit

Timestamp:

Jul 23, 2009 3:53:47 PM (15 years ago)

Author:

treat@webkit.org

Message:

2009-07-23 Jakob Petsovits <​jakob.petsovits@torchmobile.com>

Reviewed by Adam Treat.

Fix false positives for switch statement indentation check in cpplint.
​https://bugs.webkit.org/show_bug.cgi?id=27615

Makes one-line case statements (e.g. "case foo: bar();") work.
Also a few general improvements to the robustness and readability of
the check, and more test cases.

• Scripts/modules/cpplint.py:
• Scripts/modules/cpplint_unittest.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/modules/cpplint.py (modified) (2 diffs)
• Scripts/modules/cpplint_unittest.py (modified) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2009-07-23  Jakob Petsovits  <jakob.petsovits@torchmobile.com>
+
+         Reviewed by Adam Treat.
+
+         Fix false positives for switch statement indentation check in cpplint.
+         https://bugs.webkit.org/show_bug.cgi?id=27615
+
+         Makes one-line case statements (e.g. "case foo: bar();") work.
+         Also a few general improvements to the robustness and readability of
+         the check, and more test cases.
+
+         * Scripts/modules/cpplint.py:
+         * Scripts/modules/cpplint_unittest.py:
+
 2009-07-23  Jakob Petsovits  <jakob.petsovits@torchmobile.com>
 

trunk/WebKitTools/Scripts/modules/cpplint.py

@@ -1769 +1769 @@
         remaining_line = current_indentation_match.group('remaining_line')
 
-        if remaining_line.startswith('}'):
-            break # The end of the switch statement.
-        elif match(r'(default|case\s+.*)\s*:\s*$', remaining_line):
+        # End the check at the end of the switch statement.
+        if remaining_line.startswith('}') and current_indentation == switch_indentation:
+            break
+        # Case and default branches should not be indented. The regexp also
+        # catches single-line cases like "default: break;" but does not trigger
+        # on stuff like "Document::Foo();".
+        elif match(r'(default|case\s+.*)\s*:([^:].*)?$', remaining_line):
             if current_indentation != switch_indentation:
                 error(filename, line_number + line_offset, 'whitespace/indent', 4,
@@ -1778 +1782 @@
                 # one should be enough to figure out the problem.
                 break
-        elif not match(r'\w+\s*:\s*$', remaining_line):
-            # It's not a goto label (which we don't care about), so check if
-            # it's indented at least as far as the switch plus 4 spaces.
-            if not current_indentation.startswith(inner_indentation):
-                error(filename, line_number + line_offset, 'whitespace/indent', 4,
-                      'Non-label code inside switch statements should be indented.')
-                # Don't throw an error for multiple badly indented statements,
-                # one should be enough to figure out the problem.
-                break
+        # We ignore goto labels at the very beginning of a line.
+        elif match(r'\w+\s*:\s*$', remaining_line):
+            continue
+        # It's not a goto label, so check if it's indented at least as far as
+        # the switch statement plus one more level of indentation.
+        elif not current_indentation.startswith(inner_indentation):
+            error(filename, line_number + line_offset, 'whitespace/indent', 4,
+                  'Non-label code inside switch statements should be indented.')
+            # Don't throw an error for multiple badly indented statements,
+            # one should be enough to figure out the problem.
+            break
 
         if encountered_nested_switch:

trunk/WebKitTools/Scripts/modules/cpplint_unittest.py

@@ -2830 +2830 @@
         self.assert_multi_line_lint(
             '    switch (condition) {\n'
+            '    case fooCondition: break;\n'
+            '    default: return;\n'
+            '    }\n',
+            '')
+        self.assert_multi_line_lint(
+            '    switch (condition) {\n'
             '        case fooCondition:\n'
             '        case barCondition:\n'
@@ -2835 +2841 @@
             '            break;\n'
             '        default:\n'
+            '            i--;\n'
+            '    }\n',
+            'A case label should not be indented, but line up with its switch statement.'
+            '  [whitespace/indent] [4]')
+        self.assert_multi_line_lint(
+            '    switch (condition) {\n'
+            '        case fooCondition:\n'
+            '            break;\n'
+            '    default:\n'
             '            i--;\n'
             '    }\n',