Changeset 74200 in webkit


Ignore:
Timestamp:
Dec 16, 2010 10:45:43 AM (13 years ago)
Author:
levin@chromium.org
Message:

2010-12-16 David Levin <​levin@chromium.org>

Reviewed by Shinichiro Hamaji.

check-webkit-style unit tests has some duplicate boilerplate code.
​https://bugs.webkit.org/show_bug.cgi?id=49519

  • Scripts/webkitpy/style/checkers/cpp.py: (update_include_state): Replaced the "io" parameter with the global configuration _unit_test_config. This allowed not calling into functions at a low level and also not plumbing through the injection information through many levels of code. (check_for_include_what_you_use): Ditto. (process_file_data): Added the ability to set up the unit test config to allow for injection.
  • Scripts/webkitpy/style/checkers/cpp_unittest.py: (ErrorCollector.init): Added support for having a filter for errors. (ErrorCollector.call): Ditto. (CppStyleTestBase.process_file_data): Added the ability to set unit_test_config. (CppStyleTestBase.perform_lint): Consolidated logic for the perform functions. (CppStyleTestBase.perform_single_line_lint): Replace specific calls to functions in the cpp.py with generic processing and a filter that indicates what errors should be kept. (CppStyleTestBase.perform_multi_line_lint): Ditto. (CppStyleTestBase.perform_language_rules_check): Ditto. (CppStyleTestBase.perform_function_lengths_check): Ditto. (CppStyleTestBase.perform_pass_ptr_check): Ditto. (CppStyleTestBase.perform_include_what_you_use): Ditto. (CppStyleTest.test_multi_line_comments): Added another error message which applies to the test case. (CppStyleTest.test_spacing_for_binary_ops): Fixed test to not have config.h, since it is processed as a header file. (CppStyleTest.test_static_or_global_stlstrings): Fixed variable name style and indentation in checked code. (OrderOfIncludesTest.test_check_preprocessor_in_include_section): Fixed line number. (NoNonVirtualDestructorsTest.test_multi_line_declaration_with_error): Ditto.
Location:
trunk/WebKitTools
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r74163 r74200  
     12010-12-16  David Levin  <levin@chromium.org>
     2
     3        Reviewed by Shinichiro Hamaji.
     4
     5        check-webkit-style unit tests has some duplicate boilerplate code.
     6        https://bugs.webkit.org/show_bug.cgi?id=49519
     7
     8        * Scripts/webkitpy/style/checkers/cpp.py:
     9        (update_include_state): Replaced the "io" parameter with the global
     10        configuration _unit_test_config. This allowed not calling into
     11        functions at a low level and also not plumbing through the injection
     12        information through many levels of code.
     13        (check_for_include_what_you_use): Ditto.
     14        (process_file_data): Added the ability to set up the unit test config
     15        to allow for injection.
     16        * Scripts/webkitpy/style/checkers/cpp_unittest.py:
     17        (ErrorCollector.__init__): Added support for having a filter for errors.
     18        (ErrorCollector.__call__): Ditto.
     19        (CppStyleTestBase.process_file_data): Added the ability to set unit_test_config.
     20        (CppStyleTestBase.perform_lint): Consolidated logic for the perform functions.
     21        (CppStyleTestBase.perform_single_line_lint): Replace specific calls to
     22        functions in the cpp.py with generic processing and a filter that
     23        indicates what errors should be kept.
     24        (CppStyleTestBase.perform_multi_line_lint): Ditto.
     25        (CppStyleTestBase.perform_language_rules_check): Ditto.
     26        (CppStyleTestBase.perform_function_lengths_check): Ditto.
     27        (CppStyleTestBase.perform_pass_ptr_check): Ditto.
     28        (CppStyleTestBase.perform_include_what_you_use): Ditto.
     29        (CppStyleTest.test_multi_line_comments): Added another
     30        error message which applies to the test case.
     31        (CppStyleTest.test_spacing_for_binary_ops): Fixed test
     32        to not have config.h, since it is processed as a header file.
     33        (CppStyleTest.test_static_or_global_stlstrings): Fixed variable name
     34        style and indentation in checked code.
     35        (OrderOfIncludesTest.test_check_preprocessor_in_include_section):
     36        Fixed line number.
     37        (NoNonVirtualDestructorsTest.test_multi_line_declaration_with_error):
     38        Ditto.
     39
    1402010-12-15  Sheriff Bot  <webkit.review.bot@gmail.com>
    241

  • trunk/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py

    r73248 r74200  
    4848import unicodedata
    4949
     50# The key to use to provide a class to fake loading a header file.
     51INCLUDE_IO_INJECTION_KEY = 'include_header_io'
    5052
    5153# Headers that we consider STL headers.
    … …  
    117119_OTHER_HEADER = 2
    118120_MOC_HEADER = 3
     121
     122
     123# A dictionary of items customize behavior for unit test. For example,
     124# INCLUDE_IO_INJECTION_KEY allows providing a custom io class which allows
     125# for faking a header file.
     126_unit_test_config = {}
    119127
    120128
    … …  
    28252833      True if a header was succesfully added. False otherwise.
    28262834    """
     2835    io = _unit_test_config.get(INCLUDE_IO_INJECTION_KEY, codecs)
    28272836    header_file = None
    28282837    try:
    … …  
    28432852
    28442853
    2845 def check_for_include_what_you_use(filename, clean_lines, include_state, error,
    2846                                    io=codecs):
     2854def check_for_include_what_you_use(filename, clean_lines, include_state, error):
    28472855    """Reports for missing stl includes.
    28482856
    … …  
    28582866      include_state: An _IncludeState instance.
    28592867      error: The function to call with any errors found.
    2860       io: The IO factory to use to read the header file. Provided for unittest
    2861           injection.
    28622868    """
    28632869    required = {}  # A map of header name to line_number and the template entity.
    … …  
    29102916        (same_module, common_path) = files_belong_to_same_module(abs_filename, header)
    29112917        fullpath = common_path + header
    2912         if same_module and update_include_state(fullpath, include_state, io):
     2918        if same_module and update_include_state(fullpath, include_state):
    29132919            header_found = True
    29142920
    … …  
    31223128
    31233129# FIXME: Remove this function (requires refactoring unit tests).
    3124 def process_file_data(filename, file_extension, lines, error, min_confidence):
     3130def process_file_data(filename, file_extension, lines, error, min_confidence, unit_test_config):
     3131    global _unit_test_config
     3132    _unit_test_config = unit_test_config
    31253133    checker = CppChecker(filename, file_extension, error, min_confidence)
    31263134    checker.check(lines)
     3135    _unit_test_config = {}

  • trunk/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py

    r73248 r74200  
    4444import cpp as cpp_style
    4545from cpp import CppChecker
     46from ..filter import FilterConfiguration
    4647
    4748# This class works as an error collector and replaces cpp_style.Error
    … …  
    5354    _seen_style_categories = {}
    5455
    55     def __init__(self, assert_fn):
    56         """assert_fn: a function to call when we notice a problem."""
     56    def __init__(self, assert_fn, filter=None):
     57        """assert_fn: a function to call when we notice a problem.
     58           filter: filters the errors that we are concerned about."""
    5759        self._assert_fn = assert_fn
    5860        self._errors = []
     61        if not filter:
     62            filter = FilterConfiguration()
     63        self._filter = filter
    5964
    6065    def __call__(self, unused_linenum, category, confidence, message):
    … …  
    6267                        'Message "%s" has category "%s",'
    6368                        ' which is not in STYLE_CATEGORIES' % (message, category))
    64         self._seen_style_categories[category] = 1
    65         self._errors.append('%s  [%s] [%d]' % (message, category, confidence))
     69        if self._filter.should_check(category, ""):
     70            self._seen_style_categories[category] = 1
     71            self._errors.append('%s  [%s] [%d]' % (message, category, confidence))
    6672
    6773    def results(self):
    … …  
    8894                sys.exit('FATAL ERROR: There are no tests for category "%s"' % category)
    8995
    90     def remove_if_present(self, substr):
    91         for (index, error) in enumerate(self._errors):
    92             if error.find(substr) != -1:
    93                 self._errors = self._errors[0:index] + self._errors[(index + 1):]
    94                 break
    95 
    9696
    9797# This class is a lame mock of codecs. We do not verify filename, mode, or
    … …  
    131131    # Helper function to avoid needing to explicitly pass confidence
    132132    # in all the unit test calls to cpp_style.process_file_data().
    133     def process_file_data(self, filename, file_extension, lines, error):
     133    def process_file_data(self, filename, file_extension, lines, error, unit_test_config={}):
    134134        """Call cpp_style.process_file_data() with the min_confidence."""
    135135        return cpp_style.process_file_data(filename, file_extension, lines,
    136                                            error, self.min_confidence)
     136                                           error, self.min_confidence, unit_test_config)
     137
     138    def perform_lint(self, code, filename, basic_error_rules, unit_test_config={}):
     139        error_collector = ErrorCollector(self.assert_, FilterConfiguration(basic_error_rules))
     140        lines = code.split('\n')
     141        extension = filename.split('.')[1]
     142        self.process_file_data(filename, extension, lines, error_collector, unit_test_config)
     143        return error_collector.results()
    137144
    138145    # Perform lint on single line of input and return the error message.
    139     def perform_single_line_lint(self, code, file_name):
    140         error_collector = ErrorCollector(self.assert_)
    141         lines = code.split('\n')
    142         cpp_style.remove_multi_line_comments(lines, error_collector)
    143         clean_lines = cpp_style.CleansedLines(lines)
    144         include_state = cpp_style._IncludeState()
    145         function_state = cpp_style._FunctionState(self.min_confidence)
    146         ext = file_name[file_name.rfind('.') + 1:]
    147         class_state = cpp_style._ClassState()
    148         file_state = cpp_style._FileState()
    149         cpp_style.process_line(file_name, ext, clean_lines, 0,
    150                                include_state, function_state,
    151                                class_state, file_state, error_collector)
    152         # Single-line lint tests are allowed to fail the 'unlintable function'
    153         # check.
    154         error_collector.remove_if_present(
    155             'Lint failed to find start of function body.')
    156         return error_collector.results()
     146    def perform_single_line_lint(self, code, filename):
     147        basic_error_rules = ('-build/header_guard',
     148                             '-legal/copyright',
     149                             '-readability/fn_size',
     150                             '-whitespace/ending_newline')
     151        return self.perform_lint(code, filename, basic_error_rules)
    157152
    158153    # Perform lint over multiple lines and return the error message.
    159154    def perform_multi_line_lint(self, code, file_extension):
    160         error_collector = ErrorCollector(self.assert_)
    161         lines = code.split('\n')
    162         cpp_style.remove_multi_line_comments(lines, error_collector)
    163         lines = cpp_style.CleansedLines(lines)
    164         class_state = cpp_style._ClassState()
    165         file_state = cpp_style._FileState()
    166         for i in xrange(lines.num_lines()):
    167             cpp_style.check_style(lines, i, file_extension, class_state, file_state, error_collector)
    168             cpp_style.check_for_non_standard_constructs(lines, i, class_state,
    169                                                         error_collector)
    170         class_state.check_finished(error_collector)
    171         return error_collector.results()
    172 
    173     # Similar to perform_multi_line_lint, but calls check_language instead of
    174     # check_for_non_standard_constructs
    175     def perform_language_rules_check(self, file_name, code):
    176         error_collector = ErrorCollector(self.assert_)
    177         include_state = cpp_style._IncludeState()
    178         lines = code.split('\n')
    179         cpp_style.remove_multi_line_comments(lines, error_collector)
    180         lines = cpp_style.CleansedLines(lines)
    181         ext = file_name[file_name.rfind('.') + 1:]
    182         for i in xrange(lines.num_lines()):
    183             cpp_style.check_language(file_name, lines, i, ext, include_state,
    184                                      error_collector)
    185         return error_collector.results()
    186 
     155        basic_error_rules = ('-build/header_guard',
     156                             '-legal/copyright',
     157                             '-multi_line_filter',
     158                             '-whitespace/ending_newline')
     159        return self.perform_lint(code, 'test.' + file_extension, basic_error_rules)
     160
     161    # Only keep some errors related to includes, namespaces and rtti.
     162    def perform_language_rules_check(self, filename, code):
     163        basic_error_rules = ('-',
     164                             '+build/include',
     165                             '+build/include_order',
     166                             '+build/namespaces',
     167                             '+runtime/rtti')
     168        return self.perform_lint(code, filename, basic_error_rules)
     169
     170    # Only keep function length errors.
    187171    def perform_function_lengths_check(self, code):
    188         """Perform Lint function length check on block of code and return warnings.
    189 
    190         Builds up an array of lines corresponding to the code and strips comments
    191         using cpp_style functions.
    192 
    193         Establishes an error collector and invokes the function length checking
    194         function following cpp_style's pattern.
    195 
    196         Args:
    197           code: C++ source code expected to generate a warning message.
    198 
    199         Returns:
    200           The accumulated errors.
    201         """
    202         error_collector = ErrorCollector(self.assert_)
    203         function_state = cpp_style._FunctionState(self.min_confidence)
    204         lines = code.split('\n')
    205         cpp_style.remove_multi_line_comments(lines, error_collector)
    206         lines = cpp_style.CleansedLines(lines)
    207         for i in xrange(lines.num_lines()):
    208             cpp_style.detect_functions(lines, i,
    209                                        function_state, error_collector)
    210             cpp_style.check_for_function_lengths(lines, i,
    211                                                  function_state, error_collector)
    212         return error_collector.results()
    213 
    214     # Similar to perform_function_lengths_check, but calls check_pass_ptr_usage
    215     # instead of check_for_function_lengths.
     172        basic_error_rules = ('-',
     173                             '+readability/fn_size')
     174        return self.perform_lint(code, 'test.cpp', basic_error_rules)
     175
     176    # Only keep pass ptr errors.
    216177    def perform_pass_ptr_check(self, code):
    217         error_collector = ErrorCollector(self.assert_)
    218         function_state = cpp_style._FunctionState(self.min_confidence)
    219         lines = code.split('\n')
    220         cpp_style.remove_multi_line_comments(lines, error_collector)
    221         lines = cpp_style.CleansedLines(lines)
    222         for i in xrange(lines.num_lines()):
    223             cpp_style.detect_functions(lines, i,
    224                                        function_state, error_collector)
    225             cpp_style.check_pass_ptr_usage(lines, i,
    226                                            function_state, error_collector)
    227         return error_collector.results()
    228 
     178        basic_error_rules = ('-',
     179                             '+readability/pass_ptr')
     180        return self.perform_lint(code, 'test.cpp', basic_error_rules)
     181
     182    # Only include what you use errors.
    229183    def perform_include_what_you_use(self, code, filename='foo.h', io=codecs):
    230         # First, build up the include state.
    231         error_collector = ErrorCollector(self.assert_)
    232         include_state = cpp_style._IncludeState()
    233         lines = code.split('\n')
    234         cpp_style.remove_multi_line_comments(lines, error_collector)
    235         lines = cpp_style.CleansedLines(lines)
    236         file_extension = filename[filename.rfind('.') + 1:]
    237         for i in xrange(lines.num_lines()):
    238             cpp_style.check_language(filename, lines, i, file_extension, include_state,
    239                                      error_collector)
    240         # We could clear the error_collector here, but this should
    241         # also be fine, since our IncludeWhatYouUse unittests do not
    242         # have language problems.
    243 
    244         # Second, look for missing includes.
    245         cpp_style.check_for_include_what_you_use(filename, lines, include_state,
    246                                                  error_collector, io)
    247         return error_collector.results()
     184        basic_error_rules = ('-',
     185                             '+build/include_what_you_use')
     186        unit_test_config = {cpp_style.INCLUDE_IO_INJECTION_KEY: io}
     187        return self.perform_lint(code, filename, basic_error_rules, unit_test_config)
    248188
    249189    # Perform lint and compare the error message with "expected_message".
    … …  
    716656            r'''/* int a = 0; multi-liner
    717657            static const int b = 0;''',
    718       'Could not find end of multi-line comment'
    719       '  [readability/multiline_comment] [5]')
     658            ['Could not find end of multi-line comment'
     659             '  [readability/multiline_comment] [5]',
     660             'Complex multi-line /*...*/-style comment found. '
     661             'Lint may give bogus warnings.  Consider replacing these with '
     662             '//-style comments, with #if 0...#endif, or with more clearly '
     663             'structured multi-line comments.  [readability/multiline_comment] [5]'])
    720664        self.assert_multi_line_lint(r'''    /* multi-line comment''',
    721                                     'Could not find end of multi-line comment'
    722                                     '  [readability/multiline_comment] [5]')
     665                                    ['Could not find end of multi-line comment'
     666                                     '  [readability/multiline_comment] [5]',
     667                                     'Complex multi-line /*...*/-style comment found. '
     668                                     'Lint may give bogus warnings.  Consider replacing these with '
     669                                     '//-style comments, with #if 0...#endif, or with more clearly '
     670                                     'structured multi-line comments.  [readability/multiline_comment] [5]'])
    723671        self.assert_multi_line_lint(r'''    // /* comment, but not multi-line''', '')
    724672
    … …  
    13261274        self.assert_lint('if (a = b == 1)', '')
    13271275        self.assert_lint('a = 1 << 20', '')
    1328         self.assert_multi_line_lint('#include "config.h"\n#include <sys/io.h>\n',
    1329                                     '')
    1330         self.assert_multi_line_lint('#include "config.h"\n#import <foo/bar.h>\n',
    1331                                     '')
     1276        self.assert_multi_line_lint('#include <sys/io.h>\n', '')
     1277        self.assert_multi_line_lint('#import <foo/bar.h>\n', '')
    13321278
    13331279    def test_operator_methods(self):
    … …  
    13951341        self.assert_lint('string EmptyString () { return ""; }', '')
    13961342        self.assert_lint('string VeryLongNameFunctionSometimesEndsWith(\n'
    1397                          '    VeryLongNameType very_long_name_variable) {}', '')
     1343                         '    VeryLongNameType veryLongNameVariable) {}', '')
    13981344        self.assert_lint('template<>\n'
    13991345                         'string FunctionTemplateSpecialization<SomeType>(\n'
    … …  
    14061352        self.assert_lint('string Class<Type>::Method() const\n'
    14071353                         '{\n'
    1408                          '  return "";\n'
     1354                         '    return "";\n'
    14091355                         '}\n', '')
    14101356        self.assert_lint('string Class<Type>::Method(\n'
    1411                          '   int arg) const\n'
     1357                         '    int arg) const\n'
    14121358                         '{\n'
    1413                          '  return "";\n'
     1359                         '    return "";\n'
    14141360                         '}\n', '')
    14151361
    … …  
    21832129                                         '#include "foo.h"\n'
    21842130                                         '#include "g.h"\n',
    2185                                          '"foo.h" already included at foo.cpp:1  [build/include] [4]')
     2131                                         '"foo.h" already included at foo.cpp:2  [build/include] [4]')
    21862132
    21872133    def test_check_wtf_includes(self):
    … …  
    27432689             '[whitespace/braces] [4]',
    27442690             'The class Foo probably needs a virtual destructor due to having '
    2745              'virtual method(s), one declared at line 2.  [runtime/virtual] [4]'])
     2691             'virtual method(s), one declared at line 3.  [runtime/virtual] [4]'])
    27462692
    27472693
Note: See TracChangeset for help on using the changeset viewer.