Attachment 343009 Details for Bug 186796 – Patch

[patch] Patch

bug-186796-20180618194226.patch (text/plain), 3.65 KB, created by Keith Rollin on 2018-06-18 19:42:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Rollin

Created: 2018-06-18 19:42:26 PDT

Size: 3.65 KB

patch

obsolete

>Subversion Revision: 232933
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index c65cefb4ea40db1a17ede7ec78bb8a539d46280e..292ef55aafd6fbdc9e94edfa0ae9d110e376c22a 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,35 @@
>+2018-06-18  Keith Rollin  <krollin@apple.com>
>+
>+        cpp style checker needs to know more about NOLINT
>+        https://bugs.webkit.org/show_bug.cgi?id=186796
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Running check-webkit-style on
>+        ThirdParty/gtest/src/gtest-internal-inl.h crashes with a KeyError as
>+        it tries to look up information about a particular #include in a
>+        dictionary it maintains. The problem has to do with the following
>+        lines in gtest-internal-inl.h:
>+
>+        #include <gtest/gtest.h>  // NOLINT
>+        #include <gtest/gtest-spi.h>
>+
>+        The NOLINT annotation will prevent the cpp-checker from processing or
>+        even seeing that line. In skipping that line, information regarding
>+        that line is NOT added to some internal tables. However, when the code
>+        that checks that #includes are correctly alphabetized gets to the
>+        "gtest-spi.h" line, it will not respect the NOLINT annotation and will
>+        see the gtest.h line. When doing so, it runs afoul of the fact that it
>+        hadn't seen that line before and it crashes when it tries to look up
>+        that line in some internal records.
>+
>+        Fix this by catering to the possibility that a #include line may not
>+        have been entered into these internal records.
>+
>+        * Scripts/webkitpy/style/checkers/cpp.py:
>+        (check_include_line):
>+        * Scripts/webkitpy/style/checkers/cpp_unittest.py:
>+
> 2018-06-18  Zan Dobersek  <zdobersek@igalia.com>
> 
>         [webkitpy] WPTRunner should remove any metadata content before (re)generating it
>diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py
>index 39b08cad462cc503f9c9b84c3221c9b90e567dbf..5836c4f4008d290e287185205e23302b12633814 100644
>--- a/Tools/Scripts/webkitpy/style/checkers/cpp.py
>+++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py
>@@ -3070,7 +3070,7 @@ def check_include_line(filename, file_extension, clean_lines, line_number, inclu
>             previous_line_number -= 1
>             previous_line = clean_lines.lines[previous_line_number]
>             previous_match = _RE_PATTERN_INCLUDE.search(previous_line)
>-        if previous_match:
>+        if previous_match and previous_line_number in include_state.header_types:
>             previous_header_type = include_state.header_types[previous_line_number]
>             if previous_header_type == _OTHER_HEADER:
>                 if '<' in previous_line and '"' in line:
>diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
>index 782dd7e3fb32a0c1ee54c982f47c4b5a289354ca..c501a42b49690f496f2f1c355dce1fc5f2a41690 100644
>--- a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
>+++ b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
>@@ -2824,6 +2824,11 @@ class OrderOfIncludesTest(CppStyleTestBase):
>                                          '#include "array.lut.h"\n',
>                                          '')
> 
>+        self.assert_language_rules_check('foo.h',
>+                                         '#include <gtest/gtest.h> // NOLINT\n'
>+                                         '#include <gtest/gtest-spi.h>\n',
>+                                         '')
>+
>     def test_check_alphabetical_include_order_errors_reported_for_both_lines(self):
>         # If one of the two lines of out of order headers are filtered, the error should be
>         # reported on the other line.

Actions: View | Formatted Diff | Diff

Attachments on bug 186796: 343009