Remove the include guard rule in Blink C++ style
Now we use Chromium-style include guard even in Blink. Update check-webkit-style so that it doesn't accept WebKit-style include guard. Bug: 768828 Change-Id: Iea94b1857fc735fc8767b86c56156110ad6c2364 Reviewed-on: https://chromium-review.googlesource.com/1002344 Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#549153}
This commit is contained in:
styleguide/c++
third_party/WebKit/Tools/Scripts/webkitpy/style/checkers
@ -49,9 +49,9 @@ FrameLoader::FrameLoader(LocalFrame* frame)
|
||||
|
||||
## Prefer WTF types over STL types
|
||||
|
||||
Outside of `//third_party/WebKit/common`, Blink should use WTF types. STL string
|
||||
Outside of `//third_party/blink/common`, Blink should use WTF types. STL string
|
||||
and container types should only be used at the boundary to interoperate with
|
||||
'//base', `//third_party/WebKit/common`, and other Chromium-side code.
|
||||
'//base', `//third_party/blink/common`, and other Chromium-side code.
|
||||
Similarly, Blink should prefer `KURL` over `GURL` and `SecurityOrigin` over
|
||||
`url::Origin`.
|
||||
|
||||
@ -69,33 +69,6 @@ Similarly, Blink should prefer `KURL` over `GURL` and `SecurityOrigin` over
|
||||
std::unordered_map<int, std::deque<url::Origin>> origins;
|
||||
```
|
||||
|
||||
## Use Filename_h as the header include guard
|
||||
|
||||
Header include guards in Blink should match the filename exactly, replacing the
|
||||
“.” with a “\_”.
|
||||
|
||||
**Good:**
|
||||
```c++
|
||||
// third_party/WebKit/Source/core/html/HTMLDocument.h
|
||||
#ifndef HTMLDocument_h
|
||||
#define HTMLDocument_h
|
||||
|
||||
// ...
|
||||
|
||||
#endif // HTMLDocument_h
|
||||
```
|
||||
|
||||
**Bad:**
|
||||
```c++
|
||||
// third_party/WebKit/Source/core/html/HTMLDocument.h
|
||||
#ifndef THIRD_PARTY_WEBKIT_SOURCE_CORE_HTML_HTMLDOCUMENT_H_
|
||||
#define THIRD_PARTY_WEBKIT_SOURCE_CORE_HTML_HTMLDOCUMENT_H_
|
||||
|
||||
// ...
|
||||
|
||||
#endif // THIRD_PARTY_WEBKIT_SOURCE_CORE_HTML_HTMLDOCUMENT_H_
|
||||
```
|
||||
|
||||
## Naming
|
||||
|
||||
### Use `CamelCase` for all function names
|
||||
|
@ -773,35 +773,6 @@ def check_for_copyright(lines, error):
|
||||
'You should have a line: "Copyright [year] <Copyright Owner>"')
|
||||
|
||||
|
||||
# TODO(jww) After the transition of Blink into the Chromium repo, this function
|
||||
# should be removed. This will strictly enforce Chromium-style header guards,
|
||||
# rather than allowing traditional WebKit header guards and Chromium-style
|
||||
# simultaneously.
|
||||
def get_legacy_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 get_header_guard_cpp_variable(filename):
|
||||
"""Returns the CPP variable that should be used as a header guard in Chromium-style.
|
||||
|
||||
@ -817,13 +788,7 @@ def get_header_guard_cpp_variable(filename):
|
||||
# flymake.
|
||||
filename = re.sub(r'_flymake\.h$', '.h', filename)
|
||||
|
||||
# If it's a full path and starts with Source/, replace Source with blink
|
||||
# since that will be the new style directory.
|
||||
filename = sub(r'^Source\/', 'blink/', filename)
|
||||
|
||||
standard_name = sub(r'[-.\s\/]', '_', filename).upper() + '_'
|
||||
|
||||
return standard_name
|
||||
return sub(r'[-.\s\/]', '_', filename).upper() + '_'
|
||||
|
||||
|
||||
def check_for_header_guard(filename, clean_lines, error):
|
||||
@ -839,9 +804,7 @@ def check_for_header_guard(filename, clean_lines, error):
|
||||
"""
|
||||
raw_lines = clean_lines.lines_without_raw_strings
|
||||
|
||||
legacy_cpp_var = get_legacy_header_guard_cpp_variable(filename)
|
||||
cpp_var = get_header_guard_cpp_variable(filename)
|
||||
suggested_var = legacy_cpp_var[0] if re.search(r'[A-Z]', os.path.basename(filename)) else cpp_var
|
||||
|
||||
ifndef = None
|
||||
ifndef_line_number = 0
|
||||
@ -862,13 +825,13 @@ def check_for_header_guard(filename, clean_lines, error):
|
||||
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' %
|
||||
suggested_var)
|
||||
cpp_var)
|
||||
return
|
||||
|
||||
# The guard should be File_h or, for Chromium style, BLINK_PATH_TO_FILE_H_.
|
||||
if ifndef not in legacy_cpp_var and ifndef != cpp_var:
|
||||
if ifndef != cpp_var:
|
||||
error(ifndef_line_number, 'build/header_guard', 5,
|
||||
'#ifndef header guard has wrong style, please use: %s' % suggested_var)
|
||||
'#ifndef header guard has wrong style, please use: %s' % cpp_var)
|
||||
|
||||
|
||||
def check_for_unicode_replacement_characters(lines, error):
|
||||
|
@ -1420,45 +1420,17 @@ class CppStyleTest(CppStyleTestBase):
|
||||
# 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.
|
||||
# Verify that the Chromium-style header guard is allowed.
|
||||
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())
|
||||
|
||||
# Verify that the Chromium-style header guard is allowed as well.
|
||||
error_collector = ErrorCollector(self.assertTrue, header_guard_filter)
|
||||
self.process_file_data('Source/foo/testname.h', 'h',
|
||||
['#ifndef BLINK_FOO_TESTNAME_H_',
|
||||
'#define BLINK_FOO_TESTNAME_H_'],
|
||||
['#ifndef SOURCE_FOO_TESTNAME_H_',
|
||||
'#define SOURCE_FOO_TESTNAME_H_'],
|
||||
error_collector)
|
||||
self.assertEqual(0, len(error_collector.result_list()),
|
||||
error_collector.result_list())
|
||||
|
||||
# Verify that we suggest the Chromium-style header guard for a snake_case file.
|
||||
# Verify that we suggest the Chromium-style header guard.
|
||||
error_collector = ErrorCollector(self.assertTrue, header_guard_filter)
|
||||
self.process_file_data('renderer/platform/wtf/auto_reset.h', 'h',
|
||||
['#ifndef BAD_auto_reset_h', '#define BAD_auto_reset_h'],
|
||||
|
Reference in New Issue
Block a user