Bug 186796

Summary: cpp style checker needs to know more about NOLINT
Product: WebKit Reporter: Keith Rollin <krollin>
Component: Tools / TestsAssignee: Keith Rollin <krollin>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, commit-queue, ddkilzer, ews-watchlist, glenn, jbedard, lforschler, mcatanzaro, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

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.