Bug 174775

Summary: Handle case where line_numbers is None instead of an array of line numbers
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, buildbot, commit-queue, ddkilzer, glenn, jbedard, lforschler
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 173559    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Manuel Rego Casasnovas 2017-07-24 00:37:42 PDT
After r219657 (bug #173559) we're getting a weird style checker error in some patches:
* https://bugs.webkit.org/show_bug.cgi?id=174675#c5
* https://bugs.webkit.org/show_bug.cgi?id=174451#c29

Traceback (most recent call last):
  File "Tools/Scripts/check-webkit-style", line 48, in <module>
    sys.exit(CheckWebKitStyle().main())
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/style/main.py", line 155, in main
    patch_checker.check(patch)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/style/patchreader.py", line 71, in check
    self._text_file_reader.process_file(file_path=path, line_numbers=None)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/style/filereader.py", line 119, in process_file
    self._files[abs_file_path] = self._files[abs_file_path] + kwargs['line_numbers']
TypeError: can only concatenate list (not "NoneType") to list
Comment 1 Jonathan Bedard 2017-07-24 08:31:11 PDT
Created attachment 316291 [details]
Patch
Comment 2 Jonathan Bedard 2017-07-24 08:32:55 PDT
A bit surprised we didn't have a unit test for this.

It's a simple fix, added a test as well.
Comment 3 Aakash Jain 2017-07-24 09:43:37 PDT
Comment on attachment 316291 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316291&action=review

> Tools/Scripts/webkitpy/style/filereader.py:115
> +        if 'line_numbers' in kwargs and kwargs['line_numbers']:

Please check if we can simplify this to if kwargs.get('line_numbers'):

> Tools/Scripts/webkitpy/style/main_unittest.py:165
> +    def test_linter_added_file(self):

Nit: should this name be more descriptive?
Comment 4 Jonathan Bedard 2017-07-24 09:43:57 PDT
Created attachment 316299 [details]
Patch
Comment 5 Jonathan Bedard 2017-07-24 09:53:02 PDT
Created attachment 316300 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2017-07-24 10:31:15 PDT
Comment on attachment 316300 [details]
Patch for landing

Clearing flags on attachment: 316300

Committed r219827: <http://trac.webkit.org/changeset/219827>
Comment 7 WebKit Commit Bot 2017-07-24 10:31:17 PDT
All reviewed patches have been landed.  Closing bug.