diff options
| author | Simon Hausmann <simon.hausmann@digia.com> | 2012-10-17 16:21:14 +0200 |
|---|---|---|
| committer | Simon Hausmann <simon.hausmann@digia.com> | 2012-10-17 16:21:14 +0200 |
| commit | 8995b83bcbfbb68245f779b64e5517627c6cc6ea (patch) | |
| tree | 17985605dab9263cc2444bd4d45f189e142cca7c /Tools/Scripts/webkitpy/style | |
| parent | b9c9652036d5e9f1e29c574f40bc73a35c81ace6 (diff) | |
| download | qtwebkit-8995b83bcbfbb68245f779b64e5517627c6cc6ea.tar.gz | |
Imported WebKit commit cf4f8fc6f19b0629f51860cb2d4b25e139d07e00 (http://svn.webkit.org/repository/webkit/trunk@131592)
New snapshot that includes the build fixes for Mac OS X 10.6 and earlier as well
as the previously cherry-picked changes
Diffstat (limited to 'Tools/Scripts/webkitpy/style')
6 files changed, 221 insertions, 194 deletions
diff --git a/Tools/Scripts/webkitpy/style/checker.py b/Tools/Scripts/webkitpy/style/checker.py index 5b11af483..9f27c36da 100644 --- a/Tools/Scripts/webkitpy/style/checker.py +++ b/Tools/Scripts/webkitpy/style/checker.py @@ -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 diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py index a0051c979..ebbd1ad2f 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py @@ -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', diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py index a1b91f292..6f001e0cb 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py @@ -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]') diff --git a/Tools/Scripts/webkitpy/style/checkers/test_expectations.py b/Tools/Scripts/webkitpy/style/checkers/test_expectations.py index 46403b7db..1ce40cd39 100644 --- a/Tools/Scripts/webkitpy/style/checkers/test_expectations.py +++ b/Tools/Scripts/webkitpy/style/checkers/test_expectations.py @@ -67,7 +67,7 @@ class TestExpectationsChecker(object): # FIXME: host should be a required parameter, not an optional one. host = host or Host() - host._initialize_scm() + host.initialize_scm() self._port_obj = self._determine_port_from_expectations_path(host, file_path) @@ -94,8 +94,6 @@ class TestExpectationsChecker(object): expectations = '\n'.join(lines) if self._port_obj: self.check_test_expectations(expectations_str=expectations, tests=None) - else: - self._handle_style_error(1, 'test/expectations', 5, - 'No port uses path %s for test_expectations' % self._file_path) + # Warn tabs in lines as well self.check_tabs(lines) diff --git a/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py b/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py index f12397787..1516de797 100644 --- a/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py +++ b/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py @@ -82,6 +82,10 @@ class TestExpectationsTestCase(unittest.TestCase): self._expect_port_for_expectations_path('efl', 'LayoutTests/platform/efl/TestExpectations') self._expect_port_for_expectations_path('efl', 'LayoutTests/platform/efl-wk1/TestExpectations') self._expect_port_for_expectations_path('efl', 'LayoutTests/platform/efl-wk2/TestExpectations') + self._expect_port_for_expectations_path('qt', 'LayoutTests/platform/qt-win/TestExpectations') + # FIXME: check-webkit-style doesn't know how to create port objects for all Qt version (4.8, 5.0) and + # will only check files based on the installed version of Qt. + #self._expect_port_for_expectations_path('qt', 'LayoutTests/platform/qt-5.0-wk2/TestExpectations') def assert_lines_lint(self, lines, should_pass, expected_output=None): self._error_collector.reset_errors() diff --git a/Tools/Scripts/webkitpy/style/main.py b/Tools/Scripts/webkitpy/style/main.py index e90d98a42..574368a3e 100644 --- a/Tools/Scripts/webkitpy/style/main.py +++ b/Tools/Scripts/webkitpy/style/main.py @@ -124,7 +124,7 @@ class CheckWebKitStyle(object): args = sys.argv[1:] host = Host() - host._initialize_scm() + host.initialize_scm() stderr = self._engage_awesome_stderr_hacks() |
