Changeset 245870 in webkit

Timestamp:

May 29, 2019, 3:04:58 PM (7 years ago)

Author:

ddkilzer@apple.com

Message:

check-webkit-style reports false-positive build/include_order warning in WTF C++ source files
<​https://webkit.org/b/198349>

Reviewed by Alex Christensen.

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

(_classify_include): Don't return early for <wtf/Header.h>
includes.

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

(OrderOfIncludesTest.test_primary_header): Add tests for
<wtf/Header.h> includes.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/style/checkers/cpp.py (modified) (1 diff)
• Scripts/webkitpy/style/checkers/cpp_unittest.py (modified) (4 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2019-05-29  David Kilzer  <ddkilzer@apple.com>
+
+        check-webkit-style reports false-positive build/include_order warning in WTF C++ source files
+        <https://webkit.org/b/198349>
+
+        Reviewed by Alex Christensen.
+
+        * Scripts/webkitpy/style/checkers/cpp.py:
+        (_classify_include): Don't return early for <wtf/Header.h>
+        includes.
+        * Scripts/webkitpy/style/checkers/cpp_unittest.py:
+        (OrderOfIncludesTest.test_primary_header): Add tests for
+        <wtf/Header.h> includes.
+
 2019-05-29  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py

@@ -3032 +3032 @@
 
     # If it is a system header we know it is classified as _OTHER_HEADER.
-    if is_system and not include.startswith('public/'):
+    if is_system and not include.startswith('public/') and not include.startswith('wtf/'):
         return _OTHER_HEADER
 

trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py

@@ -3130 +3130 @@
                                          '#include "bar.h"\n',
                                          '')
+
         # Pretend that header files exist.
         os.path.isfile = lambda filename: True
+
         # Missing include for existing primary header -> error.
         self.assert_language_rules_check('foo.cpp',
@@ -3140 +3142 @@
                                          'Should be: config.h, primary header, blank line, and then '
                                          'alphabetically sorted.  [build/include_order] [4]')
+
         # *SoftLink.cpp files should not include their headers -> no error.
         self.assert_language_rules_check('FooSoftLink.cpp',
@@ -3146 +3149 @@
                                          '#include <wtf/SoftLinking.h>\n',
                                          '')
+
         # Having include for existing primary header -> no error.
         self.assert_language_rules_check('foo.cpp',
@@ -3153 +3157 @@
                                          '#include "bar.h"\n',
                                          '')
+
+        # Having include for existing WTF primary header -> no error.
+        self.assert_language_rules_check('foo.cpp',
+                                         '#include "config.h"\n'
+                                         '#include <wtf/foo.h>\n'
+                                         '\n'
+                                         '#include <wtf/bar.h>\n',
+                                         '')
+
+        # WTF primary header included out of order -> error.
+        self.assert_language_rules_check('foo.cpp',
+                                         '#include "config.h"\n'
+                                         '#include <wtf/bar.h>\n'
+                                         '\n'
+                                         '#include <wtf/foo.h>\n',
+                                         ['Found other header before a header this file implements. '
+                                          'Should be: config.h, primary header, blank line, and then '
+                                          'alphabetically sorted.  [build/include_order] [4]',
+                                          'Found header this file implements after other header. '
+                                          'Should be: config.h, primary header, blank line, and then '
+                                          'alphabetically sorted.  [build/include_order] [4]'])
 
         os.path.isfile = self.os_path_isfile_orig