Changeset 206897 in webkit


Ignore:
Timestamp:
Oct 6, 2016 8:26:03 PM (7 years ago)
Author:
commit-queue@webkit.org
Message:

Header guard style should be updated to be "#pragma once"
https://bugs.webkit.org/show_bug.cgi?id=159785

Patch by Joseph Pecoraro <Joseph Pecoraro> on 2016-10-06
Reviewed by Darin Adler.

  • Scripts/webkitpy/style/checkers/cpp.py:

(check_for_header_guard):
(_process_lines):
Simplify header_guard check to warn for a missing #pragma once
in header files. For legacy files that contain an #ifndef only
warn if the #ifndef line itself is changing.

  • Scripts/webkitpy/style/checkers/cpp_unittest.py:

(CppStyleTestBase.perform_header_guard_check):
(CppStyleTestBase.assert_header_guard):
Helpers for enabling just this warning.

(CppStyleTest.test_build_header_guard):
Test different header guard cases.

  • Scripts/webkitpy/style/error_handlers.py:

(DefaultStyleErrorHandler.should_line_be_checked):
Always allow warnings that output for "line 0" which won't be in
the list of modified lines that are 1-based.

Location:
trunk/Tools
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Tools/ChangeLog

    r206894 r206897  
     12016-10-06  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Header guard style should be updated to be "#pragma once"
     4        https://bugs.webkit.org/show_bug.cgi?id=159785
     5
     6        Reviewed by Darin Adler.
     7
     8        * Scripts/webkitpy/style/checkers/cpp.py:
     9        (check_for_header_guard):
     10        (_process_lines):
     11        Simplify header_guard check to warn for a missing #pragma once
     12        in header files. For legacy files that contain an #ifndef only
     13        warn if the #ifndef line itself is changing.
     14
     15        * Scripts/webkitpy/style/checkers/cpp_unittest.py:
     16        (CppStyleTestBase.perform_header_guard_check):
     17        (CppStyleTestBase.assert_header_guard):
     18        Helpers for enabling just this warning.
     19
     20        (CppStyleTest.test_build_header_guard):
     21        Test different header guard cases.
     22
     23        * Scripts/webkitpy/style/error_handlers.py:
     24        (DefaultStyleErrorHandler.should_line_be_checked):
     25        Always allow warnings that output for "line 0" which won't be in
     26        the list of modified lines that are 1-based.
     27
    1282016-10-06  Commit Queue  <commit-queue@webkit.org>
    229
  • trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py

    r205135 r206897  
    897897
    898898
    899 def get_header_guard_cpp_variable(filename):
    900     """Returns the CPP variable that should be used as a header guard.
    901 
    902     Args:
    903       filename: The name of a C++ header file.
    904 
    905     Returns:
    906       The CPP variable that should be used as a header guard in the
    907       named file.
    908 
    909     """
    910 
    911     # Restores original filename in case that style checker is invoked from Emacs's
    912     # flymake.
    913     filename = re.sub(r'_flymake\.h$', '.h', filename)
    914 
    915     standard_name = sub(r'[-.\s]', '_', os.path.basename(filename))
    916 
    917     # Files under WTF typically have header guards that start with WTF_.
    918     if '/wtf/' in filename:
    919         special_name = "WTF_" + standard_name
    920     else:
    921         special_name = standard_name
    922     return (special_name, standard_name)
    923 
    924 
    925 def check_for_header_guard(filename, lines, error):
     899def check_for_header_guard(lines, error):
    926900    """Checks that the file contains a header guard.
    927901
    928     Logs an error if no #ifndef header guard is present.  For other
    929     headers, checks that the full pathname is used.
    930 
    931     Args:
    932       filename: The name of the C++ header file.
     902    Logs an error if no #pragma once header guard is present
     903    of if there was an #ifndef guard that was modified.
     904
     905    Args:
    933906      lines: An array of strings, each representing a line of the file.
    934907      error: The function to call with any errors found.
    935908    """
    936909
    937     cppvar = get_header_guard_cpp_variable(filename)
    938 
    939     ifndef = None
     910    for line_number, line in enumerate(lines):
     911        if line.startswith('#pragma once'):
     912            return
     913
     914    # If there is no #pragma once, but there is an #ifndef, warn only if it was modified.
    940915    ifndef_line_number = 0
    941     define = None
    942916    for line_number, line in enumerate(lines):
    943917        line_split = line.split()
    944918        if len(line_split) >= 2:
    945             # find the first occurrence of #ifndef and #define, save arg
    946             if not ifndef and line_split[0] == '#ifndef':
    947                 # set ifndef to the header guard presented on the #ifndef line.
    948                 ifndef = line_split[1]
    949                 ifndef_line_number = line_number
    950             if not define and line_split[0] == '#define':
    951                 define = line_split[1]
    952             if define and ifndef:
    953                 break
    954 
    955     if not ifndef or not define or ifndef != define:
    956         error(0, 'build/header_guard', 5,
    957               'No #ifndef header guard found, suggested CPP variable is: %s' %
    958               cppvar[0])
    959         return
    960 
    961     # The guard should be File_h.
    962     if ifndef not in cppvar:
    963         error(ifndef_line_number, 'build/header_guard', 5,
    964               '#ifndef header guard has wrong style, please use: %s' % cppvar[0])
     919            if line_split[0] == '#ifndef' and line_split[1].endswith('_h'):
     920                error(line_number, 'build/header_guard', 5,
     921                    'Use #pragma once instead of #ifndef for header guard.')
     922                return
     923
     924    error(0, 'build/header_guard', 5,
     925        'Use #pragma once header guard.')
    965926
    966927
     
    38563817
    38573818    if file_extension == 'h':
    3858         check_for_header_guard(filename, lines, error)
     3819        check_for_header_guard(lines, error)
    38593820        if filename == 'Source/WTF/wtf/Platform.h':
    38603821            check_platformh_comments(lines, error)
  • trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py

    r205135 r206897  
    289289    # Only keep function length errors.
    290290    def perform_function_lengths_check(self, code):
    291         basic_error_rules = ('-',
    292                              '+readability/fn_size')
     291        basic_error_rules = ('-', '+readability/fn_size')
    293292        return self.perform_lint(code, 'test.cpp', basic_error_rules)
    294293
    295294    # Only keep pass ptr errors.
    296295    def perform_pass_ptr_check(self, code):
    297         basic_error_rules = ('-',
    298                              '+readability/pass_ptr')
     296        basic_error_rules = ('-', '+readability/pass_ptr')
    299297        return self.perform_lint(code, 'test.cpp', basic_error_rules)
    300298
    301299    # Only keep leaky pattern errors.
    302300    def perform_leaky_pattern_check(self, code):
    303         basic_error_rules = ('-',
    304                              '+runtime/leaky_pattern')
     301        basic_error_rules = ('-', '+runtime/leaky_pattern')
    305302        return self.perform_lint(code, 'test.cpp', basic_error_rules)
    306303
    307304    # Only include what you use errors.
    308305    def perform_include_what_you_use(self, code, filename='foo.h', io=codecs):
    309         basic_error_rules = ('-',
    310                              '+build/include_what_you_use')
     306        basic_error_rules = ('-', '+build/include_what_you_use')
    311307        unit_test_config = {cpp_style.INCLUDE_IO_INJECTION_KEY: io}
    312308        return self.perform_lint(code, filename, basic_error_rules, unit_test_config)
     309
     310    # Only include header guard errors.
     311    def perform_header_guard_check(self, code, filename='foo.h'):
     312        basic_error_rules = ('-', '+build/header_guard')
     313        return self.perform_lint(code, filename, basic_error_rules)
    313314
    314315    # Perform lint and compare the error message with "expected_message".
     
    341342        self.assertEqual(expected_message,
    342343                          self.perform_include_what_you_use(code))
     344
     345    def assert_header_guard(self, code, expected_message):
     346        self.assertEqual(expected_message,
     347                          self.perform_header_guard_check(code))
    343348
    344349    def assert_blank_lines_check(self, lines, start_errors, end_errors):
     
    24162421
    24172422    def test_build_header_guard(self):
    2418         file_path = 'mydir/Foo.h'
    2419 
    2420         # We can't rely on our internal stuff to get a sane path on the open source
    2421         # side of things, so just parse out the suggested header guard. This
    2422         # doesn't allow us to test the suggested header guard, but it does let us
    2423         # test all the other header tests.
    2424         error_collector = ErrorCollector(self.assertTrue)
    2425         self.process_file_data(file_path, 'h', [], error_collector)
    2426         expected_guard = ''
    2427         matcher = re.compile(
    2428             'No \#ifndef header guard found\, suggested CPP variable is\: ([A-Za-z_0-9]+) ')
    2429         for error in error_collector.result_list():
    2430             matches = matcher.match(error)
    2431             if matches:
    2432                 expected_guard = matches.group(1)
    2433                 break
    2434 
    2435         # Make sure we extracted something for our header guard.
    2436         self.assertNotEqual(expected_guard, '')
    2437 
    2438         # Wrong guard
    2439         error_collector = ErrorCollector(self.assertTrue)
    2440         self.process_file_data(file_path, 'h',
    2441                                ['#ifndef FOO_H', '#define FOO_H'], error_collector)
    2442         self.assertEqual(
    2443             1,
    2444             error_collector.result_list().count(
    2445                 '#ifndef header guard has wrong style, please use: %s'
    2446                 '  [build/header_guard] [5]' % expected_guard),
    2447             error_collector.result_list())
    2448 
    2449         # No define
    2450         error_collector = ErrorCollector(self.assertTrue)
    2451         self.process_file_data(file_path, 'h',
    2452                                ['#ifndef %s' % expected_guard], error_collector)
    2453         self.assertEqual(
    2454             1,
    2455             error_collector.result_list().count(
    2456                 'No #ifndef header guard found, suggested CPP variable is: %s'
    2457                 '  [build/header_guard] [5]' % expected_guard),
    2458             error_collector.result_list())
    2459 
    2460         # Mismatched define
    2461         error_collector = ErrorCollector(self.assertTrue)
    2462         self.process_file_data(file_path, 'h',
    2463                                ['#ifndef %s' % expected_guard,
    2464                                 '#define FOO_H'],
    2465                                error_collector)
    2466         self.assertEqual(
    2467             1,
    2468             error_collector.result_list().count(
    2469                 'No #ifndef header guard found, suggested CPP variable is: %s'
    2470                 '  [build/header_guard] [5]' % expected_guard),
    2471             error_collector.result_list())
    2472 
    2473         # No header guard errors
    2474         error_collector = ErrorCollector(self.assertTrue)
    2475         self.process_file_data(file_path, 'h',
    2476                                ['#ifndef %s' % expected_guard,
    2477                                 '#define %s' % expected_guard,
    2478                                 '#endif // %s' % expected_guard],
    2479                                error_collector)
    2480         for line in error_collector.result_list():
    2481             if line.find('build/header_guard') != -1:
    2482                 self.fail('Unexpected error: %s' % line)
    2483 
    2484         # Completely incorrect header guard
    2485         error_collector = ErrorCollector(self.assertTrue)
    2486         self.process_file_data(file_path, 'h',
    2487                                ['#ifndef FOO',
    2488                                 '#define FOO',
    2489                                 '#endif  // FOO'],
    2490                                error_collector)
    2491         self.assertEqual(
    2492             1,
    2493             error_collector.result_list().count(
    2494                 '#ifndef header guard has wrong style, please use: %s'
    2495                 '  [build/header_guard] [5]' % expected_guard),
    2496             error_collector.result_list())
    2497 
    2498         # Special case for flymake
    2499         error_collector = ErrorCollector(self.assertTrue)
    2500         self.process_file_data('mydir/Foo_flymake.h', 'h',
    2501                                ['#ifndef %s' % expected_guard,
    2502                                 '#define %s' % expected_guard,
    2503                                 '#endif // %s' % expected_guard],
    2504                                error_collector)
    2505         for line in error_collector.result_list():
    2506             if line.find('build/header_guard') != -1:
    2507                 self.fail('Unexpected error: %s' % line)
    2508 
    2509         error_collector = ErrorCollector(self.assertTrue)
    2510         self.process_file_data('mydir/Foo_flymake.h', 'h', [], error_collector)
    2511         self.assertEqual(
    2512             1,
    2513             error_collector.result_list().count(
    2514                 'No #ifndef header guard found, suggested CPP variable is: %s'
    2515                 '  [build/header_guard] [5]' % expected_guard),
    2516             error_collector.result_list())
    2517 
    2518         # Verify that we don't blindly suggest the WTF prefix for all headers.
    2519         self.assertFalse(expected_guard.startswith('WTF_'))
    2520 
    2521         # Allow the WTF_ prefix for files in that directory.
    2522         header_guard_filter = FilterConfiguration(('-', '+build/header_guard'))
    2523         error_collector = ErrorCollector(self.assertTrue, header_guard_filter)
    2524         self.process_file_data('Source/JavaScriptCore/wtf/TestName.h', 'h',
    2525                                ['#ifndef WTF_TestName_h', '#define WTF_TestName_h'],
    2526                                error_collector)
    2527         self.assertEqual(0, len(error_collector.result_list()),
    2528                           error_collector.result_list())
    2529 
    2530         # Also allow the non WTF_ prefix for files in that directory.
    2531         error_collector = ErrorCollector(self.assertTrue, header_guard_filter)
    2532         self.process_file_data('Source/JavaScriptCore/wtf/TestName.h', 'h',
    2533                                ['#ifndef TestName_h', '#define TestName_h'],
    2534                                error_collector)
    2535         self.assertEqual(0, len(error_collector.result_list()),
    2536                           error_collector.result_list())
    2537 
    2538         # Verify that we suggest the WTF prefix version.
    2539         error_collector = ErrorCollector(self.assertTrue, header_guard_filter)
    2540         self.process_file_data('Source/JavaScriptCore/wtf/TestName.h', 'h',
    2541                                ['#ifndef BAD_TestName_h', '#define BAD_TestName_h'],
    2542                                error_collector)
    2543         self.assertEqual(
    2544             1,
    2545             error_collector.result_list().count(
    2546                 '#ifndef header guard has wrong style, please use: WTF_TestName_h'
    2547                 '  [build/header_guard] [5]'),
    2548             error_collector.result_list())
     2423        rules = ('-', '+build/header_guard')
     2424
     2425        # No header guard.
     2426        self.assert_header_guard('',
     2427            'Use #pragma once header guard.'
     2428            '  [build/header_guard] [5]')
     2429
     2430        # Old header guard.
     2431        self.assert_header_guard('#ifndef Foo_h',
     2432            'Use #pragma once instead of #ifndef for header guard.'
     2433            '  [build/header_guard] [5]')
     2434
     2435        # Valid header guard.
     2436        self.assert_header_guard('#pragma once', '')
    25492437
    25502438    def test_build_printf_format(self):
  • trunk/Tools/Scripts/webkitpy/style/error_handlers.py

    r202362 r206897  
    131131        "Returns if a particular line should be checked"
    132132        # Was the line that was modified?
    133         return self._line_numbers is None or line_number in self._line_numbers
     133        return self._line_numbers is None or line_number in self._line_numbers or line_number == 0
    134134
    135135    def turn_off_line_filtering(self):
Note: See TracChangeset for help on using the changeset viewer.