Changeset 53382 in webkit


Ignore:
Timestamp:
Jan 17, 2010 6:22:42 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-01-17 Chris Jerdonek <Chris Jerdonek>

Reviewed by Shinichiro Hamaji.

Finished eliminating _cpp_style_state global state variable from
check-webkit-style code and eliminating _CppStyleState class.

https://bugs.webkit.org/show_bug.cgi?id=33764

  • Scripts/webkitpy/style/checker.py:
    • Minor updates caused by changes to cpp_style.py.
  • Scripts/webkitpy/style/cpp_style.py:
    • Removed _CppStyleState class.
    • Removed verbose_level functions.
    • Added verbosity as a parameter to _FunctionState constructor.
    • Added verbosity as a parameter to process_file().
    • Added verbosity as a parameter to process_file_data().
  • Scripts/webkitpy/style/cpp_style_unittest.py:
    • Added helper functions to set verbosity while running tests.
Location:
trunk/WebKitTools
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebKitTools/ChangeLog

    r53381 r53382  
     12010-01-17  Chris Jerdonek  <cjerdonek@webkit.org>
     2
     3        Reviewed by Shinichiro Hamaji.
     4
     5        Finished eliminating _cpp_style_state global state variable from
     6        check-webkit-style code and eliminating _CppStyleState class.
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=33764
     9
     10        * Scripts/webkitpy/style/checker.py:
     11          - Minor updates caused by changes to cpp_style.py.
     12
     13        * Scripts/webkitpy/style/cpp_style.py:
     14          - Removed _CppStyleState class.
     15          - Removed verbose_level functions.
     16          - Added verbosity as a parameter to _FunctionState constructor.
     17          - Added verbosity as a parameter to process_file().
     18          - Added verbosity as a parameter to process_file_data().
     19
     20        * Scripts/webkitpy/style/cpp_style_unittest.py:
     21          - Added helper functions to set verbosity while running tests.
     22
    1232010-01-17  Adam Barth  <abarth@webkit.org>
    224
  • trunk/WebKitTools/Scripts/webkitpy/style/checker.py

    r53374 r53382  
    646646        self.error_count = 0
    647647
    648         # FIXME: Eliminate the need to set global state here.
    649         cpp_style._set_verbose_level(options.verbosity)
    650 
    651648    def _handle_error(self, filename, line_number, category, confidence, message):
    652649        """Handle the occurrence of a style error while checking.
     
    689686        """
    690687        if cpp_style.can_handle(filename) or filename == '-':
    691             cpp_style.process_file(filename, self._handle_error)
     688            cpp_style.process_file(filename, self._handle_error, self.options.verbosity)
    692689        elif text_style.can_handle(filename):
    693690            text_style.process_file(filename, self._handle_error)
     
    721718                    self._handle_error(filename, line_number, category, confidence, message)
    722719
     720            # FIXME: Share this code with self.process_file().
    723721            if cpp_style.can_handle(filename):
    724                 cpp_style.process_file(filename, error_for_patch)
     722                cpp_style.process_file(filename, error_for_patch, self.options.verbosity)
    725723            elif text_style.can_handle(filename):
    726724                text_style.process_file(filename, error_for_patch)
  • trunk/WebKitTools/Scripts/webkitpy/style/cpp_style.py

    r53381 r53382  
    243243
    244244
    245 class _CppStyleState(object):
    246     """Maintains module-wide state.."""
    247 
    248     def __init__(self):
    249         self.verbose_level = 1  # global setting.
    250 
    251     def set_verbose_level(self, level):
    252         """Sets the module's verbosity, and returns the previous setting."""
    253         last_verbose_level = self.verbose_level
    254         self.verbose_level = level
    255         return last_verbose_level
    256 
    257 
    258 _cpp_style_state = _CppStyleState()
    259 
    260 
    261 def _verbose_level():
    262     """Returns the module's verbosity setting."""
    263     return _cpp_style_state.verbose_level
    264 
    265 
    266 def _set_verbose_level(level):
    267     """Sets the module's verbosity, and returns the previous setting."""
    268     return _cpp_style_state.set_verbose_level(level)
    269 
    270 
    271245class _FunctionState(object):
    272     """Tracks current function name and the number of lines in its body."""
     246    """Tracks current function name and the number of lines in its body.
     247
     248    Attributes:
     249      verbosity: The verbosity level to use while checking style.
     250
     251    """
    273252
    274253    _NORMAL_TRIGGER = 250  # for --v=0, 500 for --v=1, etc.
    275254    _TEST_TRIGGER = 400    # about 50% more than _NORMAL_TRIGGER.
    276255
    277     def __init__(self):
     256    def __init__(self, verbosity):
     257        self.verbosity = verbosity
    278258        self.in_a_function = False
    279259        self.lines_in_function = 0
     
    307287        else:
    308288            base_trigger = self._NORMAL_TRIGGER
    309         trigger = base_trigger * 2 ** _verbose_level()
     289        trigger = base_trigger * 2 ** self.verbosity
    310290
    311291        if self.lines_in_function > trigger:
     
    28322812
    28332813
    2834 def process_file_data(filename, file_extension, lines, error):
     2814def process_file_data(filename, file_extension, lines, error, verbosity):
    28352815    """Performs lint checks and reports any errors to the given error function.
    28362816
     
    28462826
    28472827    include_state = _IncludeState()
    2848     function_state = _FunctionState()
     2828    function_state = _FunctionState(verbosity)
    28492829    class_state = _ClassState()
    28502830    file_state = _FileState()
     
    28712851
    28722852
    2873 def process_file(filename, error):
     2853def process_file(filename, error, verbosity):
    28742854    """Performs cpp_style on a single file.
    28752855
     
    28772857      filename: The name of the file to parse.
    28782858      error: The function to call with any errors found.
     2859      verbosity: An integer that is the verbosity level to use while
     2860                 checking style.
    28792861    """
    28802862    try:
     
    29172899        sys.stderr.write('Ignoring %s; not a .cpp, .c or .h file\n' % filename)
    29182900    else:
    2919         process_file_data(filename, file_extension, lines, error)
     2901        process_file_data(filename, file_extension, lines, error, verbosity)
    29202902        if carriage_return_found and os.linesep != '\r\n':
    29212903            # Use 0 for line_number since outputing only one error for potentially
  • trunk/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py

    r53381 r53382  
    110110
    111111class CppStyleTestBase(unittest.TestCase):
    112     """Provides some useful helper functions for cpp_style tests."""
     112    """Provides some useful helper functions for cpp_style tests.
     113
     114    Attributes:
     115      verbosity: An integer that is the current verbosity level for
     116                 the tests.
     117
     118    """
     119
     120    # FIXME: Refactor the unit tests so the verbosity level is passed
     121    #        explicitly, just like it is in the real code.
     122    verbosity = 1;
     123
     124    # Helper function to avoid needing to explicitly pass verbosity
     125    # in all the unit test calls to cpp_style.process_file_data().
     126    def process_file_data(self, filename, file_extension, lines, error):
     127        """Call cpp_style.process_file_data() with the current verbosity."""
     128        return cpp_style.process_file_data(filename, file_extension, lines, error, self.verbosity)
    113129
    114130    # Perform lint on single line of input and return the error message.
     
    119135        clean_lines = cpp_style.CleansedLines(lines)
    120136        include_state = cpp_style._IncludeState()
    121         function_state = cpp_style._FunctionState()
     137        function_state = cpp_style._FunctionState(self.verbosity)
    122138        ext = file_name[file_name.rfind('.') + 1:]
    123139        class_state = cpp_style._ClassState()
     
    179195        file_name = 'foo.cpp'
    180196        error_collector = ErrorCollector(self.assert_)
    181         function_state = cpp_style._FunctionState()
     197        function_state = cpp_style._FunctionState(self.verbosity)
    182198        lines = code.split('\n')
    183199        cpp_style.remove_multi_line_comments(file_name, lines, error_collector)
     
    237253    def assert_blank_lines_check(self, lines, start_errors, end_errors):
    238254        error_collector = ErrorCollector(self.assert_)
    239         cpp_style.process_file_data('foo.cpp', 'cpp', lines, error_collector)
     255        self.process_file_data('foo.cpp', 'cpp', lines, error_collector)
    240256        self.assertEquals(
    241257            start_errors,
     
    695711
    696712        error_collector = ErrorCollector(self.assert_)
    697         cpp_style.process_file_data(file_path, 'cpp',
    698                                     ['const char* str = "This is a\\',
    699                                      ' multiline string.";'],
    700                                     error_collector)
     713        self.process_file_data(file_path, 'cpp',
     714                               ['const char* str = "This is a\\',
     715                                ' multiline string.";'],
     716                               error_collector)
    701717        self.assertEquals(
    702718            2,  # One per line.
     
    14091425        def do_test(self, data, is_missing_eof):
    14101426            error_collector = ErrorCollector(self.assert_)
    1411             cpp_style.process_file_data('foo.cpp', 'cpp', data.split('\n'),
    1412                                         error_collector)
     1427            self.process_file_data('foo.cpp', 'cpp', data.split('\n'),
     1428                                   error_collector)
    14131429            # The warning appears only once.
    14141430            self.assertEquals(
     
    14241440        def do_test(self, raw_bytes, has_invalid_utf8):
    14251441            error_collector = ErrorCollector(self.assert_)
    1426             cpp_style.process_file_data(
    1427                 'foo.cpp', 'cpp',
    1428                 unicode(raw_bytes, 'utf8', 'replace').split('\n'),
    1429                 error_collector)
     1442            self.process_file_data('foo.cpp', 'cpp',
     1443                                   unicode(raw_bytes, 'utf8', 'replace').split('\n'),
     1444                                   error_collector)
    14301445            # The warning appears only once.
    14311446            self.assertEquals(
     
    14601475    def test_allow_blank_line_before_closing_namespace(self):
    14611476        error_collector = ErrorCollector(self.assert_)
    1462         cpp_style.process_file_data('foo.cpp', 'cpp',
    1463                                     ['namespace {', '', '}  // namespace'],
    1464                                     error_collector)
     1477        self.process_file_data('foo.cpp', 'cpp',
     1478                               ['namespace {', '', '}  // namespace'],
     1479                               error_collector)
    14651480        self.assertEquals(0, error_collector.results().count(
    14661481            'Blank line at the end of a code block.  Is this needed?'
     
    14691484    def test_allow_blank_line_before_if_else_chain(self):
    14701485        error_collector = ErrorCollector(self.assert_)
    1471         cpp_style.process_file_data('foo.cpp', 'cpp',
    1472                                     ['if (hoge) {',
    1473                                      '',  # No warning
    1474                                      '} else if (piyo) {',
    1475                                      '',  # No warning
    1476                                      '} else if (piyopiyo) {',
    1477                                      '  hoge = true;',  # No warning
    1478                                      '} else {',
    1479                                      '',  # Warning on this line
    1480                                      '}'],
    1481                                     error_collector)
     1486        self.process_file_data('foo.cpp', 'cpp',
     1487                               ['if (hoge) {',
     1488                                '',  # No warning
     1489                                '} else if (piyo) {',
     1490                                '',  # No warning
     1491                                '} else if (piyopiyo) {',
     1492                                '  hoge = true;',  # No warning
     1493                                '} else {',
     1494                                '',  # Warning on this line
     1495                                '}'],
     1496                               error_collector)
    14821497        self.assertEquals(1, error_collector.results().count(
    14831498            'Blank line at the end of a code block.  Is this needed?'
     
    14861501    def test_else_on_same_line_as_closing_braces(self):
    14871502        error_collector = ErrorCollector(self.assert_)
    1488         cpp_style.process_file_data('foo.cpp', 'cpp',
    1489                                     ['if (hoge) {',
    1490                                      '',
    1491                                      '}',
    1492                                      ' else {'  # Warning on this line
    1493                                      '',
    1494                                      '}'],
    1495                                     error_collector)
     1503        self.process_file_data('foo.cpp', 'cpp',
     1504                               ['if (hoge) {',
     1505                                '',
     1506                                '}',
     1507                                ' else {'  # Warning on this line
     1508                                '',
     1509                                '}'],
     1510                               error_collector)
    14961511        self.assertEquals(1, error_collector.results().count(
    14971512            'An else should appear on the same line as the preceding }'
     
    16371652        # test all the other header tests.
    16381653        error_collector = ErrorCollector(self.assert_)
    1639         cpp_style.process_file_data(file_path, 'h', [], error_collector)
     1654        self.process_file_data(file_path, 'h', [], error_collector)
    16401655        expected_guard = ''
    16411656        matcher = re.compile(
     
    16521667        # Wrong guard
    16531668        error_collector = ErrorCollector(self.assert_)
    1654         cpp_style.process_file_data(file_path, 'h',
    1655                                     ['#ifndef FOO_H', '#define FOO_H'], error_collector)
     1669        self.process_file_data(file_path, 'h',
     1670                               ['#ifndef FOO_H', '#define FOO_H'], error_collector)
    16561671        self.assertEquals(
    16571672            1,
     
    16631678        # No define
    16641679        error_collector = ErrorCollector(self.assert_)
    1665         cpp_style.process_file_data(file_path, 'h',
    1666                                     ['#ifndef %s' % expected_guard], error_collector)
     1680        self.process_file_data(file_path, 'h',
     1681                               ['#ifndef %s' % expected_guard], error_collector)
    16671682        self.assertEquals(
    16681683            1,
     
    16741689        # Mismatched define
    16751690        error_collector = ErrorCollector(self.assert_)
    1676         cpp_style.process_file_data(file_path, 'h',
    1677                                     ['#ifndef %s' % expected_guard,
    1678                                      '#define FOO_H'],
    1679                                     error_collector)
     1691        self.process_file_data(file_path, 'h',
     1692                               ['#ifndef %s' % expected_guard,
     1693                                '#define FOO_H'],
     1694                               error_collector)
    16801695        self.assertEquals(
    16811696            1,
     
    16871702        # No header guard errors
    16881703        error_collector = ErrorCollector(self.assert_)
    1689         cpp_style.process_file_data(file_path, 'h',
    1690                                     ['#ifndef %s' % expected_guard,
    1691                                      '#define %s' % expected_guard,
    1692                                      '#endif // %s' % expected_guard],
    1693                                     error_collector)
     1704        self.process_file_data(file_path, 'h',
     1705                               ['#ifndef %s' % expected_guard,
     1706                                '#define %s' % expected_guard,
     1707                                '#endif // %s' % expected_guard],
     1708                               error_collector)
    16941709        for line in error_collector.result_list():
    16951710            if line.find('build/header_guard') != -1:
     
    16981713        # Completely incorrect header guard
    16991714        error_collector = ErrorCollector(self.assert_)
    1700         cpp_style.process_file_data(file_path, 'h',
    1701                                     ['#ifndef FOO',
    1702                                      '#define FOO',
    1703                                      '#endif  // FOO'],
    1704                                     error_collector)
     1715        self.process_file_data(file_path, 'h',
     1716                               ['#ifndef FOO',
     1717                                '#define FOO',
     1718                                '#endif  // FOO'],
     1719                               error_collector)
    17051720        self.assertEquals(
    17061721            1,
     
    18441859        # There should be a copyright message in the first 10 lines
    18451860        error_collector = ErrorCollector(self.assert_)
    1846         cpp_style.process_file_data(file_path, 'cpp', [], error_collector)
     1861        self.process_file_data(file_path, 'cpp', [], error_collector)
    18471862        self.assertEquals(
    18481863            1,
     
    18501865
    18511866        error_collector = ErrorCollector(self.assert_)
    1852         cpp_style.process_file_data(
     1867        self.process_file_data(
    18531868            file_path, 'cpp',
    18541869            ['' for unused_i in range(10)] + [copyright_line],
     
    18601875        # Test that warning isn't issued if Copyright line appears early enough.
    18611876        error_collector = ErrorCollector(self.assert_)
    1862         cpp_style.process_file_data(file_path, 'cpp', [copyright_line], error_collector)
     1877        self.process_file_data(file_path, 'cpp', [copyright_line], error_collector)
    18631878        for message in error_collector.result_list():
    18641879            if message.find('legal/copyright') != -1:
     
    18661881
    18671882        error_collector = ErrorCollector(self.assert_)
    1868         cpp_style.process_file_data(
     1883        self.process_file_data(
    18691884            file_path, 'cpp',
    18701885            ['' for unused_i in range(9)] + [copyright_line],
     
    22012216        cpp_style._FunctionState._TEST_TRIGGER = self.old_test_trigger
    22022217
     2218    # FIXME: Eliminate the need for this function.
     2219    def set_verbosity(self, verbosity):
     2220        """Set new test verbosity and return old test verbosity."""
     2221        old_verbosity = self.verbosity
     2222        self.verbosity = verbosity
     2223        return old_verbosity
     2224
    22032225    def assert_function_lengths_check(self, code, expected_message):
    22042226        """Check warnings for long function bodies are as expected.
     
    22402262          error_level:  --v setting for cpp_style.
    22412263        """
    2242         trigger_level = self.trigger_lines(cpp_style._verbose_level())
     2264        trigger_level = self.trigger_lines(self.verbosity)
    22432265        self.assert_function_lengths_check(
    22442266            'void test(int x)' + self.function_body(lines),
     
    23232345
    23242346    def test_function_length_check_definition_below_severity0(self):
    2325         old_verbosity = cpp_style._set_verbose_level(0)
     2347        old_verbosity = self.set_verbosity(0)
    23262348        self.assert_function_length_check_definition_ok(self.trigger_lines(0) - 1)
    2327         cpp_style._set_verbose_level(old_verbosity)
     2349        self.set_verbosity(old_verbosity)
    23282350
    23292351    def test_function_length_check_definition_at_severity0(self):
    2330         old_verbosity = cpp_style._set_verbose_level(0)
     2352        old_verbosity = self.set_verbosity(0)
    23312353        self.assert_function_length_check_definition_ok(self.trigger_lines(0))
    2332         cpp_style._set_verbose_level(old_verbosity)
     2354        self.set_verbosity(old_verbosity)
    23332355
    23342356    def test_function_length_check_definition_above_severity0(self):
    2335         old_verbosity = cpp_style._set_verbose_level(0)
     2357        old_verbosity = self.set_verbosity(0)
    23362358        self.assert_function_length_check_above_error_level(0)
    2337         cpp_style._set_verbose_level(old_verbosity)
     2359        self.set_verbosity(old_verbosity)
    23382360
    23392361    def test_function_length_check_definition_below_severity1v0(self):
    2340         old_verbosity = cpp_style._set_verbose_level(0)
     2362        old_verbosity = self.set_verbosity(0)
    23412363        self.assert_function_length_check_below_error_level(1)
    2342         cpp_style._set_verbose_level(old_verbosity)
     2364        self.set_verbosity(old_verbosity)
    23432365
    23442366    def test_function_length_check_definition_at_severity1v0(self):
    2345         old_verbosity = cpp_style._set_verbose_level(0)
     2367        old_verbosity = self.set_verbosity(0)
    23462368        self.assert_function_length_check_at_error_level(1)
    2347         cpp_style._set_verbose_level(old_verbosity)
     2369        self.set_verbosity(old_verbosity)
    23482370
    23492371    def test_function_length_check_definition_below_severity1(self):
     
    23592381        error_level = 1
    23602382        error_lines = self.trigger_lines(error_level) + 1
    2361         trigger_level = self.trigger_lines(cpp_style._verbose_level())
     2383        trigger_level = self.trigger_lines(self.verbosity)
    23622384        self.assert_function_lengths_check(
    23632385            'void test_blanks(int x)' + self.function_body(error_lines),
     
    23712393        error_level = 1
    23722394        error_lines = self.trigger_lines(error_level) + 1
    2373         trigger_level = self.trigger_lines(cpp_style._verbose_level())
     2395        trigger_level = self.trigger_lines(self.verbosity)
    23742396        self.assert_function_lengths_check(
    23752397            ('my_namespace::my_other_namespace::MyVeryLongTypeName*\n'
     
    23862408        error_level = 1
    23872409        error_lines = self.trigger_test_lines(error_level) + 1
    2388         trigger_level = self.trigger_test_lines(cpp_style._verbose_level())
     2410        trigger_level = self.trigger_test_lines(self.verbosity)
    23892411        self.assert_function_lengths_check(
    23902412            'TEST_F(Test, Mutator)' + self.function_body(error_lines),
     
    23982420        error_level = 1
    23992421        error_lines = self.trigger_test_lines(error_level) + 1
    2400         trigger_level = self.trigger_test_lines(cpp_style._verbose_level())
     2422        trigger_level = self.trigger_test_lines(self.verbosity)
    24012423        self.assert_function_lengths_check(
    24022424            ('TEST_F(GoogleUpdateRecoveryRegistryProtectedTest,\n'
     
    24132435        error_level = 1
    24142436        error_lines = self.trigger_test_lines(error_level) + 1
    2415         trigger_level = self.trigger_test_lines(cpp_style._verbose_level())
     2437        trigger_level = self.trigger_test_lines(self.verbosity)
    24162438        self.assert_function_lengths_check(
    24172439            ('TEST_F('
     
    24262448        error_level = 1
    24272449        error_lines = self.trigger_lines(error_level) + 1
    2428         trigger_level = self.trigger_lines(cpp_style._verbose_level())
     2450        trigger_level = self.trigger_lines(self.verbosity)
    24292451        self.assert_function_lengths_check(
    24302452            'void test(int x)' + self.function_body_with_no_lints(error_lines),
Note: See TracChangeset for help on using the changeset viewer.