Skip to content
Permalink
Browse files
Teach style checker about preprocessor directive indentation rules
https://bugs.webkit.org/show_bug.cgi?id=96874

Reviewed by Adam Barth.

Preprocessor directives (#ifdef, #include, #define, etc.) should not be indented.
This is not explicit in our style guide but is generally followed in our code.
Searching for violations in our codebase shows these are rarely indented:
    #include - indented in 6 files
    #ifdef - indented in 0 files
    #ifndef - indented in 1 file
    #define - indented in 11 files
    #if - indented in 7 files

* Scripts/webkitpy/style/checkers/cpp.py:
(check_directive_indentation):

    This is the simple test where we look for spaces followed by a #.

(check_style):
* Scripts/webkitpy/style/checkers/cpp_unittest.py:

    A few tests needed to be modified because they had unintentionally indented
    preprocessor directives.

(CppStyleTest.test_build_class.Foo):
(CppStyleTest.test_build_class):
(CppStyleTest.test_build_class.DERIVE_FROM_GOO):
(WebKitStyleTest.test_line_breaking):
(WebKitStyleTest.test_directive_indentation):


Canonical link: https://commits.webkit.org/114854@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@128782 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
progers committed Sep 17, 2012
1 parent a42af3b commit fa9b6e4cfdfbebfc095a5d70ce13de39c0111526
Showing 3 changed files with 68 additions and 8 deletions.
@@ -1,3 +1,36 @@
2012-09-17 Philip Rogers <pdr@google.com>

Teach style checker about preprocessor directive indentation rules
https://bugs.webkit.org/show_bug.cgi?id=96874

Reviewed by Adam Barth.

Preprocessor directives (#ifdef, #include, #define, etc.) should not be indented.
This is not explicit in our style guide but is generally followed in our code.
Searching for violations in our codebase shows these are rarely indented:
#include - indented in 6 files
#ifdef - indented in 0 files
#ifndef - indented in 1 file
#define - indented in 11 files
#if - indented in 7 files

* Scripts/webkitpy/style/checkers/cpp.py:
(check_directive_indentation):

This is the simple test where we look for spaces followed by a #.

(check_style):
* Scripts/webkitpy/style/checkers/cpp_unittest.py:

A few tests needed to be modified because they had unintentionally indented
preprocessor directives.

(CppStyleTest.test_build_class.Foo):
(CppStyleTest.test_build_class):
(CppStyleTest.test_build_class.DERIVE_FROM_GOO):
(WebKitStyleTest.test_line_breaking):
(WebKitStyleTest.test_directive_indentation):

2012-09-17 Tor Arne Vestbø <tor.arne.vestbo@nokia.com>

[Qt] Auto-generate the module pri file for QtWebKit
@@ -2040,6 +2040,26 @@ def check_namespace_indentation(clean_lines, line_number, file_extension, file_s
break;


def check_directive_indentation(clean_lines, line_number, file_state, error):
"""Looks for indentation of preprocessor directives.
Args:
clean_lines: A CleansedLines instance containing the file.
line_number: The number of the line to check.
file_state: A _FileState instance which maintains information about
the state of things in the file.
error: The function to call with any errors found.
"""

line = clean_lines.elided[line_number] # Get rid of comments and strings.

indented_preprocessor_directives = match(r'\s+#', line)
if not indented_preprocessor_directives:
return

error(line_number, 'whitespace/indent', 4, 'preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.')


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;'.
@@ -2581,6 +2601,7 @@ def check_style(clean_lines, line_number, file_extension, class_state, file_stat

# Some more style checks
check_namespace_indentation(clean_lines, line_number, file_extension, file_state, error)
check_directive_indentation(clean_lines, line_number, file_state, error)
check_using_std(clean_lines, line_number, file_state, error)
check_max_min_macros(clean_lines, line_number, file_state, error)
check_ctype_functions(clean_lines, line_number, file_state, error)
@@ -2086,13 +2086,13 @@ def test_build_class(self):
# Here is an example where the linter gets confused, even though
# the code doesn't violate the style guide.
self.assert_multi_line_lint(
'''class Foo
#ifdef DERIVE_FROM_GOO
: public Goo {
#else
: public Hoo {
#endif
};''',
'class Foo\n'
'#ifdef DERIVE_FROM_GOO\n'
' : public Goo {\n'
'#else\n'
' : public Hoo {\n'
'#endif\n'
'};',
'Failed to find complete declaration of class Foo'
' [build/class] [5]')

@@ -3821,7 +3821,7 @@ def test_line_breaking(self):
'More than one command on the same line [whitespace/newline] [4]')
# Ignore preprocessor if's.
self.assert_multi_line_lint(
' #if (condition) || (condition2)\n',
'#if (condition) || (condition2)\n',
'')

# 2. An else statement should go on the same line as a preceding
@@ -4391,6 +4391,12 @@ def test_null_false_zero(self):
'if (UNLIKELY(foo == NULL))',
'Use 0 instead of NULL. [readability/null] [5]')

def test_directive_indentation(self):
self.assert_lint(
" #if FOO",
"preprocessor directives (e.g., #ifdef, #define, #import) should never be indented."
" [whitespace/indent] [4]",
"foo.cpp")

def test_using_std(self):
self.assert_lint(

0 comments on commit fa9b6e4

Please sign in to comment.