diff options
Diffstat (limited to 'Tools/Scripts/webkitpy/style/checkers')
4 files changed, 90 insertions, 39 deletions
diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py index 250b9ee..671fd56 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py @@ -194,6 +194,31 @@ def iteratively_replace_matches_with_char(pattern, char_replacement, s): s = s[:start_match_index] + char_replacement * match_length + s[end_match_index:] +def _rfind_in_lines(regex, lines, start_position, not_found_position): + """Does a reverse find starting at start position and going backwards until + a match is found. + + Returns the position where the regex ended. + """ + # Put the regex in a group and proceed it with a greedy expression that + # matches anything to ensure that we get the last possible match in a line. + last_in_line_regex = r'.*(' + regex + ')' + current_row = start_position.row + + # Start with the given row and trim off everything past what may be matched. + current_line = lines[start_position.row][:start_position.column] + while True: + found_match = match(last_in_line_regex, current_line) + if found_match: + return Position(current_row, found_match.end(1)) + + # A match was not found so continue backward. + current_row -= 1 + if current_row < 0: + return not_found_position + current_line = lines[current_row] + + def _convert_to_lower_with_underscores(text): """Converts all text strings in camelCase or PascalCase to lowers with underscores.""" @@ -526,6 +551,15 @@ class _FunctionState(object): self._clean_lines = clean_lines self._parameter_list = None + def modifiers_and_return_type(self): + """Returns the modifiers and the return type.""" + # Go backwards from where the function name is until we encounter one of several things: + # ';' or '{' or '}' or 'private:', etc. or '#' or return Position(0, 0) + elided = self._clean_lines.elided + start_modifiers = _rfind_in_lines(r';|\{|\}|((private|public|protected):)|(#.*)', + elided, self.parameter_start_position, Position(0, 0)) + return SingleLineView(elided, start_modifiers, self.function_name_start_position).single_line.strip() + def parameter_list(self): if not self._parameter_list: # Store the final result as a tuple since that is immutable. @@ -2315,7 +2349,7 @@ def check_for_null(clean_lines, line_number, file_state, error): # matches, then do the check with strings collapsed to avoid giving errors for # NULLs occurring in strings. if search(r'\bNULL\b', line) and search(r'\bNULL\b', CleansedLines.collapse_strings(line)): - error(line_number, 'readability/null', 4, 'Use 0 instead of NULL.') + error(line_number, 'readability/null', 4, 'Use 0 or null instead of NULL (even in *comments*).') def get_line_width(line): """Determines the width of the line in column positions. diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py index 868d3f6..53670d7 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py @@ -352,16 +352,17 @@ class CppStyleTestBase(unittest.TestCase): class FunctionDetectionTest(CppStyleTestBase): - def perform_function_detection(self, lines, function_information): + def perform_function_detection(self, lines, function_information, detection_line=0): clean_lines = cpp_style.CleansedLines(lines) function_state = cpp_style._FunctionState(5) error_collector = ErrorCollector(self.assert_) - cpp_style.detect_functions(clean_lines, 0, function_state, error_collector) + cpp_style.detect_functions(clean_lines, detection_line, function_state, error_collector) if not function_information: self.assertEquals(function_state.in_a_function, False) return self.assertEquals(function_state.in_a_function, True) self.assertEquals(function_state.current_function, function_information['name'] + '()') + self.assertEquals(function_state.modifiers_and_return_type(), function_information['modifiers_and_return_type']) self.assertEquals(function_state.is_pure, function_information['is_pure']) self.assertEquals(function_state.is_declaration, function_information['is_declaration']) self.assert_positions_equal(function_state.function_name_start_position, function_information['function_name_start_position']) @@ -385,6 +386,7 @@ class FunctionDetectionTest(CppStyleTestBase): ['void theTestFunctionName(int) {', '}'], {'name': 'theTestFunctionName', + 'modifiers_and_return_type': 'void', 'function_name_start_position': (0, 5), 'parameter_start_position': (0, 24), 'parameter_end_position': (0, 29), @@ -397,6 +399,7 @@ class FunctionDetectionTest(CppStyleTestBase): self.perform_function_detection( ['void aFunctionName(int);'], {'name': 'aFunctionName', + 'modifiers_and_return_type': 'void', 'function_name_start_position': (0, 5), 'parameter_start_position': (0, 18), 'parameter_end_position': (0, 23), @@ -408,6 +411,7 @@ class FunctionDetectionTest(CppStyleTestBase): self.perform_function_detection( ['CheckedInt<T> operator /(const CheckedInt<T> &lhs, const CheckedInt<T> &rhs);'], {'name': 'operator /', + 'modifiers_and_return_type': 'CheckedInt<T>', 'function_name_start_position': (0, 14), 'parameter_start_position': (0, 24), 'parameter_end_position': (0, 76), @@ -419,6 +423,7 @@ class FunctionDetectionTest(CppStyleTestBase): self.perform_function_detection( ['CheckedInt<T> operator -(const CheckedInt<T> &lhs, const CheckedInt<T> &rhs);'], {'name': 'operator -', + 'modifiers_and_return_type': 'CheckedInt<T>', 'function_name_start_position': (0, 14), 'parameter_start_position': (0, 24), 'parameter_end_position': (0, 76), @@ -430,6 +435,7 @@ class FunctionDetectionTest(CppStyleTestBase): self.perform_function_detection( ['CheckedInt<T> operator !=(const CheckedInt<T> &lhs, const CheckedInt<T> &rhs);'], {'name': 'operator !=', + 'modifiers_and_return_type': 'CheckedInt<T>', 'function_name_start_position': (0, 14), 'parameter_start_position': (0, 25), 'parameter_end_position': (0, 77), @@ -441,6 +447,7 @@ class FunctionDetectionTest(CppStyleTestBase): self.perform_function_detection( ['CheckedInt<T> operator +(const CheckedInt<T> &lhs, const CheckedInt<T> &rhs);'], {'name': 'operator +', + 'modifiers_and_return_type': 'CheckedInt<T>', 'function_name_start_position': (0, 14), 'parameter_start_position': (0, 24), 'parameter_end_position': (0, 76), @@ -453,6 +460,7 @@ class FunctionDetectionTest(CppStyleTestBase): self.perform_function_detection( ['virtual void theTestFunctionName(int = 0);'], {'name': 'theTestFunctionName', + 'modifiers_and_return_type': 'virtual void', 'function_name_start_position': (0, 13), 'parameter_start_position': (0, 32), 'parameter_end_position': (0, 41), @@ -464,6 +472,7 @@ class FunctionDetectionTest(CppStyleTestBase): self.perform_function_detection( ['virtual void theTestFunctionName(int) = 0;'], {'name': 'theTestFunctionName', + 'modifiers_and_return_type': 'virtual void', 'function_name_start_position': (0, 13), 'parameter_start_position': (0, 32), 'parameter_end_position': (0, 37), @@ -478,6 +487,7 @@ class FunctionDetectionTest(CppStyleTestBase): ' = ', ' 0 ;'], {'name': 'theTestFunctionName', + 'modifiers_and_return_type': 'virtual void', 'function_name_start_position': (0, 13), 'parameter_start_position': (0, 32), 'parameter_end_position': (0, 37), @@ -498,6 +508,7 @@ class FunctionDetectionTest(CppStyleTestBase): # This isn't a function but it looks like one to our simple # algorithm and that is ok. {'name': 'asm', + 'modifiers_and_return_type': '', 'function_name_start_position': (0, 0), 'parameter_start_position': (0, 3), 'parameter_end_position': (2, 1), @@ -514,6 +525,7 @@ class FunctionDetectionTest(CppStyleTestBase): function_state = self.perform_function_detection( ['void functionName();'], {'name': 'functionName', + 'modifiers_and_return_type': 'void', 'function_name_start_position': (0, 5), 'parameter_start_position': (0, 17), 'parameter_end_position': (0, 19), @@ -527,6 +539,7 @@ class FunctionDetectionTest(CppStyleTestBase): function_state = self.perform_function_detection( ['void functionName(int);'], {'name': 'functionName', + 'modifiers_and_return_type': 'void', 'function_name_start_position': (0, 5), 'parameter_start_position': (0, 17), 'parameter_end_position': (0, 22), @@ -541,6 +554,7 @@ class FunctionDetectionTest(CppStyleTestBase): function_state = self.perform_function_detection( ['void functionName(unsigned a, short b, long c, long long short unsigned int);'], {'name': 'functionName', + 'modifiers_and_return_type': 'void', 'function_name_start_position': (0, 5), 'parameter_start_position': (0, 17), 'parameter_end_position': (0, 76), @@ -558,6 +572,7 @@ class FunctionDetectionTest(CppStyleTestBase): function_state = self.perform_function_detection( ['virtual void determineARIADropEffects(Vector<String>*&, const unsigned long int*&, const MediaPlayer::Preload, Other<Other2, Other3<P1, P2> >, int);'], {'name': 'determineARIADropEffects', + 'modifiers_and_return_type': 'virtual void', 'parameter_start_position': (0, 37), 'function_name_start_position': (0, 13), 'parameter_end_position': (0, 147), @@ -574,23 +589,27 @@ class FunctionDetectionTest(CppStyleTestBase): # Try parsing a function with a very complex definition. function_state = self.perform_function_detection( - ['AnotherTemplate<Class1, Class2> aFunctionName(PassRefPtr<MyClass> paramName,', + ['#define MyMacro(a) a', + 'virtual', + 'AnotherTemplate<Class1, Class2> aFunctionName(PassRefPtr<MyClass> paramName,', 'const Other1Class& foo,', 'const ComplexTemplate<Class1, NestedTemplate<P1, P2> >* const * param = new ComplexTemplate<Class1, NestedTemplate<P1, P2> >(34, 42),', 'int* myCount = 0);'], {'name': 'aFunctionName', - 'function_name_start_position': (0, 32), - 'parameter_start_position': (0, 45), - 'parameter_end_position': (3, 17), - 'body_start_position': (3, 17), - 'end_position': (3, 18), + 'modifiers_and_return_type': 'virtual AnotherTemplate<Class1, Class2>', + 'function_name_start_position': (2, 32), + 'parameter_start_position': (2, 45), + 'parameter_end_position': (5, 17), + 'body_start_position': (5, 17), + 'end_position': (5, 18), 'is_pure': False, 'is_declaration': True, 'parameter_list': - ({'type': 'PassRefPtr<MyClass>', 'name': 'paramName', 'row': 0}, - {'type': 'const Other1Class&', 'name': 'foo', 'row': 1}, - {'type': 'const ComplexTemplate<Class1, NestedTemplate<P1, P2> >* const *', 'name': 'param', 'row': 2}, - {'type': 'int*', 'name': 'myCount', 'row': 3})}) + ({'type': 'PassRefPtr<MyClass>', 'name': 'paramName', 'row': 2}, + {'type': 'const Other1Class&', 'name': 'foo', 'row': 3}, + {'type': 'const ComplexTemplate<Class1, NestedTemplate<P1, P2> >* const *', 'name': 'param', 'row': 4}, + {'type': 'int*', 'name': 'myCount', 'row': 5})}, + detection_line=2) class CppStyleTest(CppStyleTestBase): @@ -630,6 +649,14 @@ class CppStyleTest(CppStyleTestBase): self.assertTrue(position < cpp_style.Position(position.row + 1, position.column - 1)) self.assertEquals(position.__str__(), '(3, 4)') + def test_rfind_in_lines(self): + not_found_position = cpp_style.Position(10, 11) + start_position = cpp_style.Position(2, 2) + lines = ['ab', 'ace', 'test'] + self.assertEquals(not_found_position, cpp_style._rfind_in_lines('st', lines, start_position, not_found_position)) + self.assertTrue(cpp_style.Position(1, 1) == cpp_style._rfind_in_lines('a', lines, start_position, not_found_position)) + self.assertEquals(cpp_style.Position(2, 2), cpp_style._rfind_in_lines('(te|a)', lines, start_position, not_found_position)) + def test_close_expression(self): self.assertEquals(cpp_style.Position(1, -1), cpp_style.close_expression([')('], cpp_style.Position(0, 1))) self.assertEquals(cpp_style.Position(1, -1), cpp_style.close_expression([') ()'], cpp_style.Position(0, 1))) @@ -3901,12 +3928,12 @@ class WebKitStyleTest(CppStyleTestBase): 'foo.cpp') self.assert_lint( "// Don't use NULL in comments since it isn't in code.", - 'Use 0 instead of NULL.' + 'Use 0 or null instead of NULL (even in *comments*).' ' [readability/null] [4]', 'foo.cpp') self.assert_lint( '"A string with NULL" // and a comment with NULL is tricky to flag correctly in cpp_style.', - 'Use 0 instead of NULL.' + 'Use 0 or null instead of NULL (even in *comments*).' ' [readability/null] [4]', 'foo.cpp') self.assert_lint( diff --git a/Tools/Scripts/webkitpy/style/checkers/test_expectations.py b/Tools/Scripts/webkitpy/style/checkers/test_expectations.py index c86b32c..e37f908 100644 --- a/Tools/Scripts/webkitpy/style/checkers/test_expectations.py +++ b/Tools/Scripts/webkitpy/style/checkers/test_expectations.py @@ -75,7 +75,6 @@ class TestExpectationsChecker(object): "Using 'test' port, but platform-specific expectations " "will fail the check." % self._file_path) self._port_obj = port.get('test') - self._port_to_check = self._port_obj.test_platform_name() # Suppress error messages of test_expectations module since they will be # reported later. log = logging.getLogger("webkitpy.layout_tests.layout_package." @@ -91,7 +90,7 @@ class TestExpectationsChecker(object): try: expectations = test_expectations.TestExpectationsFile( port=self._port_obj, expectations=expectations_str, full_test_list=tests, - test_platform_name=self._port_to_check, is_debug_mode=False, + test_config=self._port_obj.test_configuration(), is_lint_mode=True, overrides=overrides) except test_expectations.ParseError, error: err = error diff --git a/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py b/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py index 9817c5d..f0813e1 100644 --- a/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py +++ b/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py @@ -84,15 +84,6 @@ class TestExpectationsTestCase(unittest.TestCase): def test_valid_expectations(self): self.assert_lines_lint( - ["passes/text.html = PASS"], - "") - self.assert_lines_lint( - ["passes/text.html = FAIL PASS"], - "") - self.assert_lines_lint( - ["passes/text.html = CRASH TIMEOUT FAIL PASS"], - "") - self.assert_lines_lint( ["BUGCR1234 MAC : passes/text.html = PASS FAIL"], "") self.assert_lines_lint( @@ -120,12 +111,12 @@ class TestExpectationsTestCase(unittest.TestCase): def test_modifier_errors(self): self.assert_lines_lint( ["BUG1234 : passes/text.html = FAIL"], - 'Bug must be either BUGCR, BUGWK, or BUGV8_ for test: bug1234 passes/text.html [test/expectations] [5]') + "BUG\\d+ is not allowed, must be one of BUGCR\\d+, BUGWK\\d+, BUGV8_\\d+, or a non-numeric bug identifier. passes/text.html [test/expectations] [5]") def test_valid_modifiers(self): self.assert_lines_lint( ["INVALID-MODIFIER : passes/text.html = PASS"], - "Invalid modifier for test: invalid-modifier " + "Unrecognized option 'invalid-modifier' " "passes/text.html [test/expectations] [5]") self.assert_lines_lint( ["SKIP : passes/text.html = PASS"], @@ -135,38 +126,38 @@ class TestExpectationsTestCase(unittest.TestCase): def test_expectation_errors(self): self.assert_lines_lint( ["missing expectations"], - "Missing expectations. ['missing expectations'] [test/expectations] [5]") + "Missing a ':' missing expectations [test/expectations] [5]") self.assert_lines_lint( ["SLOW : passes/text.html = TIMEOUT"], - "A test can not be both slow and timeout. " - "If it times out indefinitely, then it should be just timeout. " + "A test can not be both SLOW and TIMEOUT. " + "If it times out indefinitely, then it should be just TIMEOUT. " "passes/text.html [test/expectations] [5]") self.assert_lines_lint( - ["does/not/exist.html = FAIL"], + ["BUGWK1 : does/not/exist.html = FAIL"], "Path does not exist. does/not/exist.html [test/expectations] [2]") def test_parse_expectations(self): self.assert_lines_lint( - ["passes/text.html = PASS"], + ["BUGWK1 : passes/text.html = PASS"], "") self.assert_lines_lint( - ["passes/text.html = UNSUPPORTED"], + ["BUGWK1 : passes/text.html = UNSUPPORTED"], "Unsupported expectation: unsupported " "passes/text.html [test/expectations] [5]") self.assert_lines_lint( - ["passes/text.html = PASS UNSUPPORTED"], + ["BUGWK1 : passes/text.html = PASS UNSUPPORTED"], "Unsupported expectation: unsupported " "passes/text.html [test/expectations] [5]") def test_already_seen_test(self): self.assert_lines_lint( - ["passes/text.html = PASS", - "passes/text.html = TIMEOUT"], - "Duplicate expectation. %s [test/expectations] [5]" % self._test_file) + ["BUGWK1 : passes/text.html = PASS", + "BUGWK1 : passes/text.html = TIMEOUT"], + "Duplicate or ambiguous expectation. %s [test/expectations] [5]" % self._test_file) def test_tab(self): self.assert_lines_lint( - ["\tpasses/text.html = PASS"], + ["\tBUGWK1 : passes/text.html = PASS"], "Line contains tab character. [whitespace/tab] [5]") if __name__ == '__main__': |