<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[206897] trunk/Tools</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/206897">206897</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-10-06 20:26:03 -0700 (Thu, 06 Oct 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Header guard style should be updated to be "#pragma once"
https://bugs.webkit.org/show_bug.cgi?id=159785
Patch by Joseph Pecoraro <pecoraro@apple.com> 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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsScriptswebkitpystylecheckerscpppy">trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py</a></li>
<li><a href="#trunkToolsScriptswebkitpystylecheckerscpp_unittestpy">trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py</a></li>
<li><a href="#trunkToolsScriptswebkitpystyleerror_handlerspy">trunk/Tools/Scripts/webkitpy/style/error_handlers.py</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (206896 => 206897)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2016-10-07 03:21:27 UTC (rev 206896)
+++ trunk/Tools/ChangeLog 2016-10-07 03:26:03 UTC (rev 206897)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2016-10-06 Joseph Pecoraro <pecoraro@apple.com>
+
+ Header guard style should be updated to be "#pragma once"
+ https://bugs.webkit.org/show_bug.cgi?id=159785
+
+ 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.
+
</ins><span class="cx"> 2016-10-06 Commit Queue <commit-queue@webkit.org>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling out r206713.
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpystylecheckerscpppy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py (206896 => 206897)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2016-10-07 03:21:27 UTC (rev 206896)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2016-10-07 03:26:03 UTC (rev 206897)
</span><span class="lines">@@ -896,74 +896,35 @@
</span><span class="cx"> 'You should have a line: "Copyright [year] <Copyright Owner>"')
</span><span class="cx">
</span><span class="cx">
</span><del>-def get_header_guard_cpp_variable(filename):
- """Returns the CPP variable that should be used as a header guard.
-
- Args:
- filename: The name of a C++ header file.
-
- Returns:
- The CPP variable that should be used as a header guard in the
- named file.
-
- """
-
- # Restores original filename in case that style checker is invoked from Emacs's
- # flymake.
- filename = re.sub(r'_flymake\.h$', '.h', filename)
-
- standard_name = sub(r'[-.\s]', '_', os.path.basename(filename))
-
- # Files under WTF typically have header guards that start with WTF_.
- if '/wtf/' in filename:
- special_name = "WTF_" + standard_name
- else:
- special_name = standard_name
- return (special_name, standard_name)
-
-
-def check_for_header_guard(filename, lines, error):
</del><ins>+def check_for_header_guard(lines, error):
</ins><span class="cx"> """Checks that the file contains a header guard.
</span><span class="cx">
</span><del>- Logs an error if no #ifndef header guard is present. For other
- headers, checks that the full pathname is used.
</del><ins>+ Logs an error if no #pragma once header guard is present
+ of if there was an #ifndef guard that was modified.
</ins><span class="cx">
</span><span class="cx"> Args:
</span><del>- filename: The name of the C++ header file.
</del><span class="cx"> lines: An array of strings, each representing a line of the file.
</span><span class="cx"> error: The function to call with any errors found.
</span><span class="cx"> """
</span><span class="cx">
</span><del>- cppvar = get_header_guard_cpp_variable(filename)
</del><ins>+ for line_number, line in enumerate(lines):
+ if line.startswith('#pragma once'):
+ return
</ins><span class="cx">
</span><del>- ifndef = None
</del><ins>+ # If there is no #pragma once, but there is an #ifndef, warn only if it was modified.
</ins><span class="cx"> ifndef_line_number = 0
</span><del>- define = None
</del><span class="cx"> for line_number, line in enumerate(lines):
</span><span class="cx"> line_split = line.split()
</span><span class="cx"> if len(line_split) >= 2:
</span><del>- # find the first occurrence of #ifndef and #define, save arg
- if not ifndef and line_split[0] == '#ifndef':
- # set ifndef to the header guard presented on the #ifndef line.
- ifndef = line_split[1]
- ifndef_line_number = line_number
- if not define and line_split[0] == '#define':
- define = line_split[1]
- if define and ifndef:
- break
</del><ins>+ if line_split[0] == '#ifndef' and line_split[1].endswith('_h'):
+ error(line_number, 'build/header_guard', 5,
+ 'Use #pragma once instead of #ifndef for header guard.')
+ return
</ins><span class="cx">
</span><del>- if not ifndef or not define or ifndef != define:
- error(0, 'build/header_guard', 5,
- 'No #ifndef header guard found, suggested CPP variable is: %s' %
- cppvar[0])
- return
</del><ins>+ error(0, 'build/header_guard', 5,
+ 'Use #pragma once header guard.')
</ins><span class="cx">
</span><del>- # The guard should be File_h.
- if ifndef not in cppvar:
- error(ifndef_line_number, 'build/header_guard', 5,
- '#ifndef header guard has wrong style, please use: %s' % cppvar[0])
</del><span class="cx">
</span><del>-
</del><span class="cx"> def check_for_unicode_replacement_characters(lines, error):
</span><span class="cx"> """Logs an error for each line containing Unicode replacement characters.
</span><span class="cx">
</span><span class="lines">@@ -3855,7 +3816,7 @@
</span><span class="cx"> check_for_copyright(lines, error)
</span><span class="cx">
</span><span class="cx"> if file_extension == 'h':
</span><del>- check_for_header_guard(filename, lines, error)
</del><ins>+ check_for_header_guard(lines, error)
</ins><span class="cx"> if filename == 'Source/WTF/wtf/Platform.h':
</span><span class="cx"> check_platformh_comments(lines, error)
</span><span class="cx">
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpystylecheckerscpp_unittestpy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py (206896 => 206897)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2016-10-07 03:21:27 UTC (rev 206896)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2016-10-07 03:26:03 UTC (rev 206897)
</span><span class="lines">@@ -288,29 +288,30 @@
</span><span class="cx">
</span><span class="cx"> # Only keep function length errors.
</span><span class="cx"> def perform_function_lengths_check(self, code):
</span><del>- basic_error_rules = ('-',
- '+readability/fn_size')
</del><ins>+ basic_error_rules = ('-', '+readability/fn_size')
</ins><span class="cx"> return self.perform_lint(code, 'test.cpp', basic_error_rules)
</span><span class="cx">
</span><span class="cx"> # Only keep pass ptr errors.
</span><span class="cx"> def perform_pass_ptr_check(self, code):
</span><del>- basic_error_rules = ('-',
- '+readability/pass_ptr')
</del><ins>+ basic_error_rules = ('-', '+readability/pass_ptr')
</ins><span class="cx"> return self.perform_lint(code, 'test.cpp', basic_error_rules)
</span><span class="cx">
</span><span class="cx"> # Only keep leaky pattern errors.
</span><span class="cx"> def perform_leaky_pattern_check(self, code):
</span><del>- basic_error_rules = ('-',
- '+runtime/leaky_pattern')
</del><ins>+ basic_error_rules = ('-', '+runtime/leaky_pattern')
</ins><span class="cx"> return self.perform_lint(code, 'test.cpp', basic_error_rules)
</span><span class="cx">
</span><span class="cx"> # Only include what you use errors.
</span><span class="cx"> def perform_include_what_you_use(self, code, filename='foo.h', io=codecs):
</span><del>- basic_error_rules = ('-',
- '+build/include_what_you_use')
</del><ins>+ basic_error_rules = ('-', '+build/include_what_you_use')
</ins><span class="cx"> unit_test_config = {cpp_style.INCLUDE_IO_INJECTION_KEY: io}
</span><span class="cx"> return self.perform_lint(code, filename, basic_error_rules, unit_test_config)
</span><span class="cx">
</span><ins>+ # Only include header guard errors.
+ def perform_header_guard_check(self, code, filename='foo.h'):
+ basic_error_rules = ('-', '+build/header_guard')
+ return self.perform_lint(code, filename, basic_error_rules)
+
</ins><span class="cx"> # Perform lint and compare the error message with "expected_message".
</span><span class="cx"> def assert_lint(self, code, expected_message, file_name='foo.cpp'):
</span><span class="cx"> self.assertEqual(expected_message, self.perform_single_line_lint(code, file_name))
</span><span class="lines">@@ -341,6 +342,10 @@
</span><span class="cx"> self.assertEqual(expected_message,
</span><span class="cx"> self.perform_include_what_you_use(code))
</span><span class="cx">
</span><ins>+ def assert_header_guard(self, code, expected_message):
+ self.assertEqual(expected_message,
+ self.perform_header_guard_check(code))
+
</ins><span class="cx"> def assert_blank_lines_check(self, lines, start_errors, end_errors):
</span><span class="cx"> error_collector = ErrorCollector(self.assertTrue)
</span><span class="cx"> self.process_file_data('foo.cpp', 'cpp', lines, error_collector)
</span><span class="lines">@@ -2415,138 +2420,21 @@
</span><span class="cx"> ' [build/forward_decl] [5]')
</span><span class="cx">
</span><span class="cx"> def test_build_header_guard(self):
</span><del>- file_path = 'mydir/Foo.h'
</del><ins>+ rules = ('-', '+build/header_guard')
</ins><span class="cx">
</span><del>- # We can't rely on our internal stuff to get a sane path on the open source
- # side of things, so just parse out the suggested header guard. This
- # doesn't allow us to test the suggested header guard, but it does let us
- # test all the other header tests.
- error_collector = ErrorCollector(self.assertTrue)
- self.process_file_data(file_path, 'h', [], error_collector)
- expected_guard = ''
- matcher = re.compile(
- 'No \#ifndef header guard found\, suggested CPP variable is\: ([A-Za-z_0-9]+) ')
- for error in error_collector.result_list():
- matches = matcher.match(error)
- if matches:
- expected_guard = matches.group(1)
- break
</del><ins>+ # No header guard.
+ self.assert_header_guard('',
+ 'Use #pragma once header guard.'
+ ' [build/header_guard] [5]')
</ins><span class="cx">
</span><del>- # Make sure we extracted something for our header guard.
- self.assertNotEqual(expected_guard, '')
</del><ins>+ # Old header guard.
+ self.assert_header_guard('#ifndef Foo_h',
+ 'Use #pragma once instead of #ifndef for header guard.'
+ ' [build/header_guard] [5]')
</ins><span class="cx">
</span><del>- # Wrong guard
- error_collector = ErrorCollector(self.assertTrue)
- self.process_file_data(file_path, 'h',
- ['#ifndef FOO_H', '#define FOO_H'], error_collector)
- self.assertEqual(
- 1,
- error_collector.result_list().count(
- '#ifndef header guard has wrong style, please use: %s'
- ' [build/header_guard] [5]' % expected_guard),
- error_collector.result_list())
</del><ins>+ # Valid header guard.
+ self.assert_header_guard('#pragma once', '')
</ins><span class="cx">
</span><del>- # No define
- error_collector = ErrorCollector(self.assertTrue)
- self.process_file_data(file_path, 'h',
- ['#ifndef %s' % expected_guard], error_collector)
- self.assertEqual(
- 1,
- error_collector.result_list().count(
- 'No #ifndef header guard found, suggested CPP variable is: %s'
- ' [build/header_guard] [5]' % expected_guard),
- error_collector.result_list())
-
- # Mismatched define
- error_collector = ErrorCollector(self.assertTrue)
- self.process_file_data(file_path, 'h',
- ['#ifndef %s' % expected_guard,
- '#define FOO_H'],
- error_collector)
- self.assertEqual(
- 1,
- error_collector.result_list().count(
- 'No #ifndef header guard found, suggested CPP variable is: %s'
- ' [build/header_guard] [5]' % expected_guard),
- error_collector.result_list())
-
- # No header guard errors
- error_collector = ErrorCollector(self.assertTrue)
- self.process_file_data(file_path, 'h',
- ['#ifndef %s' % expected_guard,
- '#define %s' % expected_guard,
- '#endif // %s' % expected_guard],
- error_collector)
- for line in error_collector.result_list():
- if line.find('build/header_guard') != -1:
- self.fail('Unexpected error: %s' % line)
-
- # Completely incorrect header guard
- error_collector = ErrorCollector(self.assertTrue)
- self.process_file_data(file_path, 'h',
- ['#ifndef FOO',
- '#define FOO',
- '#endif // FOO'],
- error_collector)
- self.assertEqual(
- 1,
- error_collector.result_list().count(
- '#ifndef header guard has wrong style, please use: %s'
- ' [build/header_guard] [5]' % expected_guard),
- error_collector.result_list())
-
- # Special case for flymake
- error_collector = ErrorCollector(self.assertTrue)
- self.process_file_data('mydir/Foo_flymake.h', 'h',
- ['#ifndef %s' % expected_guard,
- '#define %s' % expected_guard,
- '#endif // %s' % expected_guard],
- error_collector)
- for line in error_collector.result_list():
- if line.find('build/header_guard') != -1:
- self.fail('Unexpected error: %s' % line)
-
- error_collector = ErrorCollector(self.assertTrue)
- self.process_file_data('mydir/Foo_flymake.h', 'h', [], error_collector)
- self.assertEqual(
- 1,
- error_collector.result_list().count(
- 'No #ifndef header guard found, suggested CPP variable is: %s'
- ' [build/header_guard] [5]' % expected_guard),
- error_collector.result_list())
-
- # Verify that we don't blindly suggest the WTF prefix for all headers.
- self.assertFalse(expected_guard.startswith('WTF_'))
-
- # Allow the WTF_ prefix for files in that directory.
- header_guard_filter = FilterConfiguration(('-', '+build/header_guard'))
- error_collector = ErrorCollector(self.assertTrue, header_guard_filter)
- self.process_file_data('Source/JavaScriptCore/wtf/TestName.h', 'h',
- ['#ifndef WTF_TestName_h', '#define WTF_TestName_h'],
- error_collector)
- self.assertEqual(0, len(error_collector.result_list()),
- error_collector.result_list())
-
- # Also allow the non WTF_ prefix for files in that directory.
- error_collector = ErrorCollector(self.assertTrue, header_guard_filter)
- self.process_file_data('Source/JavaScriptCore/wtf/TestName.h', 'h',
- ['#ifndef TestName_h', '#define TestName_h'],
- error_collector)
- self.assertEqual(0, len(error_collector.result_list()),
- error_collector.result_list())
-
- # Verify that we suggest the WTF prefix version.
- error_collector = ErrorCollector(self.assertTrue, header_guard_filter)
- self.process_file_data('Source/JavaScriptCore/wtf/TestName.h', 'h',
- ['#ifndef BAD_TestName_h', '#define BAD_TestName_h'],
- error_collector)
- self.assertEqual(
- 1,
- error_collector.result_list().count(
- '#ifndef header guard has wrong style, please use: WTF_TestName_h'
- ' [build/header_guard] [5]'),
- error_collector.result_list())
-
</del><span class="cx"> def test_build_printf_format(self):
</span><span class="cx"> self.assert_lint(
</span><span class="cx"> r'printf("\%%d", value);',
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpystyleerror_handlerspy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/style/error_handlers.py (206896 => 206897)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/webkitpy/style/error_handlers.py 2016-10-07 03:21:27 UTC (rev 206896)
+++ trunk/Tools/Scripts/webkitpy/style/error_handlers.py 2016-10-07 03:26:03 UTC (rev 206897)
</span><span class="lines">@@ -130,7 +130,7 @@
</span><span class="cx"> def should_line_be_checked(self, line_number):
</span><span class="cx"> "Returns if a particular line should be checked"
</span><span class="cx"> # Was the line that was modified?
</span><del>- return self._line_numbers is None or line_number in self._line_numbers
</del><ins>+ return self._line_numbers is None or line_number in self._line_numbers or line_number == 0
</ins><span class="cx">
</span><span class="cx"> def turn_off_line_filtering(self):
</span><span class="cx"> self._line_numbers = None
</span></span></pre>
</div>
</div>
</body>
</html>