diff options
Diffstat (limited to 'Tools/Scripts/webkitpy/style')
4 files changed, 129 insertions, 21 deletions
diff --git a/Tools/Scripts/webkitpy/style/checker.py b/Tools/Scripts/webkitpy/style/checker.py index 23b9ab3..975432b 100644 --- a/Tools/Scripts/webkitpy/style/checker.py +++ b/Tools/Scripts/webkitpy/style/checker.py @@ -123,14 +123,11 @@ _PATH_RULES_SPECIFIER = [ # The API test harnesses have no config.h and use funny macros like # TEST_CLASS_NAME. "Tools/WebKitAPITest/", - "Tools/TestWebKitAPI/"], + "Tools/TestWebKitAPI/", + "Source/WebKit/qt/tests/qdeclarativewebview"], ["-build/include", "-readability/naming"]), - ([# The EFL APIs use EFL naming style, which includes - # both lower-cased and camel-cased, underscore-sparated - # values. - "Source/WebKit/efl/ewk/", - # There is no clean way to avoid "yy_*" names used by flex. + ([# There is no clean way to avoid "yy_*" names used by flex. "Source/WebCore/css/CSSParser.cpp", # Qt code uses '_' in some places (such as private slots # and on test xxx_data methos on tests) @@ -144,13 +141,16 @@ _PATH_RULES_SPECIFIER = [ "Tools/MiniBrowser/qt"], ["-build/include"]), ([# The GTK+ APIs use GTK+ naming style, which includes - # lower-cased, underscore-separated values. + # lower-cased, underscore-separated values, whitespace before + # parens for function calls, and always having variable names. # Also, GTK+ allows the use of NULL. "Source/WebCore/bindings/scripts/test/GObject", - "WebKit/gtk/webkit/", + "Source/WebKit/gtk/webkit/", "Tools/DumpRenderTree/gtk/"], ["-readability/naming", - "-readability/null"]), + "-readability/parameter_name", + "-readability/null", + "-whitespace/parens"]), ([# Header files in ForwardingHeaders have no header guards or # exceptional header guards (e.g., WebCore_FWD_Debugger_h). "/ForwardingHeaders/"], @@ -164,6 +164,13 @@ _PATH_RULES_SPECIFIER = [ ["-readability/parameter_name", "-whitespace/parens"]), + ([# The EFL APIs use EFL naming style, which includes + # both lower-cased and camel-cased, underscore-sparated + # values. + "Source/WebKit/efl/ewk/"], + ["-readability/naming", + "-readability/parameter_name"]), + # WebKit2 rules: # WebKit2 and certain directories have idiosyncracies. ([# NPAPI has function names with underscores. @@ -253,7 +260,7 @@ _SKIPPED_FILES_WITH_WARNING = [ # This list should be in addition to files with FileType.NONE. Files # with FileType.NONE are automatically skipped without warning. _SKIPPED_FILES_WITHOUT_WARNING = [ - "LayoutTests/", + "LayoutTests" + os.path.sep, ] # Extensions of files which are allowed to contain carriage returns. @@ -475,7 +482,7 @@ class CheckerDispatcher(object): elif file_extension in _XML_FILE_EXTENSIONS: return FileType.XML elif (os.path.basename(file_path).startswith('ChangeLog') or - (not file_extension and "Tools/Scripts/" in file_path) or + (not file_extension and os.path.join("Tools", "Scripts") in file_path) or file_extension in _TEXT_FILE_EXTENSIONS): return FileType.TEXT else: diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py index 671fd56..7f8a9ea 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py @@ -1565,21 +1565,40 @@ def _check_parameter_name_against_text(parameter, text, error): return True -def check_function_definition(clean_lines, line_number, function_state, error): +def check_function_definition(filename, file_extension, clean_lines, line_number, function_state, error): """Check that function definitions for style issues. Specifically, check that parameter names in declarations add information. Args: + filename: Filename of the file that is being processed. + file_extension: The current file extension, without the leading dot. clean_lines: A CleansedLines instance containing the file. line_number: The number of the line to check. function_state: Current function name and lines in body so far. error: The function to call with any errors found. """ - # Only do checks when we have a function declaration. - if line_number != function_state.body_start_position.row or not function_state.is_declaration: + if line_number != function_state.body_start_position.row: return + modifiers_and_return_type = function_state.modifiers_and_return_type() + if filename.find('/chromium/') != -1 and search(r'\bWEBKIT_API\b', modifiers_and_return_type): + if filename.find('/chromium/public/') == -1: + error(function_state.function_name_start_position.row, 'readability/webkit_api', 5, + 'WEBKIT_API should only appear in the chromium public directory.') + elif not file_extension == "h": + error(function_state.function_name_start_position.row, 'readability/webkit_api', 5, + 'WEBKIT_API should only be used in header files.') + elif not function_state.is_declaration or search(r'\binline\b', modifiers_and_return_type): + error(function_state.function_name_start_position.row, 'readability/webkit_api', 5, + 'WEBKIT_API should not be used on a function with a body.') + elif function_state.is_pure: + error(function_state.function_name_start_position.row, 'readability/webkit_api', 5, + 'WEBKIT_API should not be used with a pure virtual function.') + + # Do checks specific to function declaractions. + if not function_state.is_declaration: + return parameter_list = function_state.parameter_list() for parameter in parameter_list: if not parameter.name: @@ -1707,7 +1726,7 @@ def check_spacing(file_extension, clean_lines, line_number, error): error(line_number, 'whitespace/blank_line', 3, 'Blank line at the end of a code block. Is this needed?') - # Next, we complain if there's a comment too near the text + # Next, we check for proper spacing with respect to comments. comment_position = line.find('//') if comment_position != -1: # Check if the // may be in quotes. If so, ignore it @@ -1735,6 +1754,11 @@ def check_spacing(file_extension, clean_lines, line_number, error): error(line_number, 'whitespace/comments', 4, 'Should have a space between // and comment') + # There should only be one space after punctuation in a comment. + if search('[.!?,;:]\s\s', line[comment_position:]): + error(line_number, 'whitespace/comments', 5, + 'Should only a single space after a punctuation in a comment.') + line = clean_lines.elided[line_number] # get rid of comments and strings # Don't try to do spacing checks for operator methods @@ -2108,7 +2132,8 @@ def check_braces(clean_lines, line_number, error): # We check if a closed brace has started a line to see if a # one line control statement was previous. previous_line = clean_lines.elided[line_number - 2] - if (previous_line.find('{') > 0 and previous_line.find('}') < 0 + last_open_brace = previous_line.rfind('{') + if (last_open_brace != -1 and previous_line.find('}', last_open_brace) == -1 and search(r'\b(if|for|foreach|while|else)\b', previous_line)): error(line_number, 'whitespace/braces', 4, 'One line control clauses should not use braces.') @@ -3313,7 +3338,7 @@ def process_line(filename, file_extension, check_for_function_lengths(clean_lines, line, function_state, error) if search(r'\bNOLINT\b', raw_lines[line]): # ignore nolint lines return - check_function_definition(clean_lines, line, function_state, error) + check_function_definition(filename, file_extension, clean_lines, line, function_state, error) check_pass_ptr_usage(clean_lines, line, function_state, error) check_for_multiline_comments_and_strings(clean_lines, line, error) check_style(clean_lines, line, file_extension, class_state, file_state, error) @@ -3404,6 +3429,7 @@ class CppChecker(object): 'readability/streams', 'readability/todo', 'readability/utf8', + 'readability/webkit_api', 'runtime/arrays', 'runtime/casting', 'runtime/explicit', diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py index 53670d7..2d2abbf 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py @@ -1002,9 +1002,9 @@ class CppStyleTest(CppStyleTestBase): mock_header_contents = [''] message = self.perform_include_what_you_use( '''#include "config.h" - #include "%s/a.h" + #include "%s%sa.h" - std::set<int> foo;''' % os.path.basename(os.getcwd()), + std::set<int> foo;''' % (os.path.basename(os.getcwd()), os.path.sep), filename='a.cpp', io=MockIo(mock_header_contents)) self.assertEquals(message, 'Add #include <set> for set<> ' @@ -1779,7 +1779,7 @@ class CppStyleTest(CppStyleTestBase): self.assert_multi_line_lint('#endif\n );', '') - def test_two_spaces_between_code_and_comments(self): + def test_one_spaces_between_code_and_comments(self): self.assert_lint('} // namespace foo', '') self.assert_lint('}// namespace foo', @@ -1806,6 +1806,24 @@ class CppStyleTest(CppStyleTestBase): self.assert_lint('printf("\\"%s // In quotes.")', '') self.assert_lint('printf("%s", "// In quotes.")', '') + def test_one_spaces_after_punctuation_in_comments(self): + self.assert_lint('int a; // This is a sentence.', + '') + self.assert_lint('int a; // This is a sentence. This is a another sentence.', + '') + self.assert_lint('int a; // This is a sentence. This is a another sentence.', + 'Should only a single space after a punctuation in a comment. [whitespace/comments] [5]') + self.assert_lint('int a; // This is a sentence! This is a another sentence.', + 'Should only a single space after a punctuation in a comment. [whitespace/comments] [5]') + self.assert_lint('int a; // Why did I write this? This is a another sentence.', + 'Should only a single space after a punctuation in a comment. [whitespace/comments] [5]') + self.assert_lint('int a; // Elementary, my dear.', + 'Should only a single space after a punctuation in a comment. [whitespace/comments] [5]') + self.assert_lint('int a; // The following should be clear: Is it?', + 'Should only a single space after a punctuation in a comment. [whitespace/comments] [5]') + self.assert_lint('int a; // Look at the follow semicolon; I hope this gives an error.', + 'Should only a single space after a punctuation in a comment. [whitespace/comments] [5]') + def test_space_after_comment_marker(self): self.assert_lint('//', '') self.assert_lint('//x', 'Should have a space between // and comment' @@ -3616,6 +3634,21 @@ class WebKitStyleTest(CppStyleTestBase): ['More than one command on the same line in if [whitespace/parens] [4]', 'One line control clauses should not use braces. [whitespace/braces] [4]']) self.assert_multi_line_lint( + 'if (condition)\n' + ' doSomething();\n' + 'else {\n' + ' doSomethingElse();\n' + '}\n', + 'One line control clauses should not use braces. [whitespace/braces] [4]') + self.assert_multi_line_lint( + 'if (condition) {\n' + ' doSomething1();\n' + ' doSomething2();\n' + '} else {\n' + ' doSomethingElse();\n' + '}\n', + 'One line control clauses should not use braces. [whitespace/braces] [4]') + self.assert_multi_line_lint( 'void func()\n' '{\n' ' while (condition) { }\n' @@ -4332,6 +4365,48 @@ class WebKitStyleTest(CppStyleTestBase): 'One space before end of line comments' ' [whitespace/comments] [5]') + def test_webkit_api_check(self): + webkit_api_error_rules = ('-', + '+readability/webkit_api') + self.assertEquals('', + self.perform_lint('WEBKIT_API int foo();\n', + 'WebKit/chromium/public/test.h', + webkit_api_error_rules)) + self.assertEquals('WEBKIT_API should only be used in header files. [readability/webkit_api] [5]', + self.perform_lint('WEBKIT_API int foo();\n', + 'WebKit/chromium/public/test.cpp', + webkit_api_error_rules)) + self.assertEquals('WEBKIT_API should only appear in the chromium public directory. [readability/webkit_api] [5]', + self.perform_lint('WEBKIT_API int foo();\n', + 'WebKit/chromium/src/test.h', + webkit_api_error_rules)) + self.assertEquals('WEBKIT_API should not be used on a function with a body. [readability/webkit_api] [5]', + self.perform_lint('WEBKIT_API int foo() { }\n', + 'WebKit/chromium/public/test.h', + webkit_api_error_rules)) + self.assertEquals('WEBKIT_API should not be used on a function with a body. [readability/webkit_api] [5]', + self.perform_lint('WEBKIT_API inline int foo()\n' + '{\n' + '}\n', + 'WebKit/chromium/public/test.h', + webkit_api_error_rules)) + self.assertEquals('WEBKIT_API should not be used with a pure virtual function. [readability/webkit_api] [5]', + self.perform_lint('{}\n' + 'WEBKIT_API\n' + 'virtual\n' + 'int\n' + 'foo() = 0;\n', + 'WebKit/chromium/public/test.h', + webkit_api_error_rules)) + self.assertEquals('', + self.perform_lint('{}\n' + 'WEBKIT_API\n' + 'virtual\n' + 'int\n' + 'foo() = 0;\n', + 'test.h', + webkit_api_error_rules)) + def test_other(self): # FIXME: Implement this. pass diff --git a/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py b/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py index f0813e1..9d03acc 100644 --- a/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py +++ b/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py @@ -67,7 +67,7 @@ class TestExpectationsTestCase(unittest.TestCase): def setUp(self): self._error_collector = ErrorCollector() port_obj = port.get('test') - self._test_file = os.path.join(port_obj.layout_tests_dir(), 'passes/text.html') + self._test_file = port_obj._filesystem.join(port_obj.layout_tests_dir(), 'passes/text.html') def process_expectations(self, expectations, overrides=None): self._checker = TestExpectationsChecker() |