Bug 186796 - cpp style checker needs to know more about NOLINT
Summary: cpp style checker needs to know more about NOLINT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Rollin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-18 19:28 PDT by Keith Rollin
Modified: 2018-06-19 14:58 PDT (History)
9 users (show)

See Also:


Attachments
Patch (3.65 KB, patch)
2018-06-18 19:42 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Rollin 2018-06-18 19:28:26 PDT
Running check-webkit-style on ThirdParty/gtest/src/gtest-internal-inl.h crashes with a traceback like:

  Traceback (most recent call last):
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 2830, in test_check_alphabetical_include_order
      '')
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 339, in assert_language_rules_check
      self.perform_language_rules_check(file_name, code, lines_to_check))
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 287, in perform_language_rules_check
      return self.perform_lint(code, filename, basic_error_rules, lines_to_check=lines_to_check)
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 259, in perform_lint
      self.process_file_data(filename, extension, lines, error_collector, unit_test_config)
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 253, in process_file_data
      checker.check(lines)
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 4030, in check
      self.handle_style_error, self.min_confidence)
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3884, in _process_lines
      enum_state, asm_state, error)
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3831, in process_line
      file_state, error)
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3122, in check_language
      check_include_line(filename, file_extension, clean_lines, line_number, include_state, error)
    File "/Volumes/Data/dev/webkit/OpenSource/Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3075, in check_include_line
      previous_header_type = include_state.header_types[previous_line_number]
  KeyError: 65
Comment 1 Keith Rollin 2018-06-18 19:29:01 PDT
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.
Comment 2 Keith Rollin 2018-06-18 19:42:26 PDT
Created attachment 343009 [details]
Patch
Comment 3 Radar WebKit Bug Importer 2018-06-19 12:48:05 PDT
<rdar://problem/41260116>
Comment 4 Jonathan Bedard 2018-06-19 14:14:46 PDT
Unofficial r=me.
Comment 5 WebKit Commit Bot 2018-06-19 14:58:37 PDT
Comment on attachment 343009 [details]
Patch

Clearing flags on attachment: 343009

Committed r232986: <https://trac.webkit.org/changeset/232986>
Comment 6 WebKit Commit Bot 2018-06-19 14:58:38 PDT
All reviewed patches have been landed.  Closing bug.