Bug 174775 - Handle case where line_numbers is None instead of an array of line numbers
Summary: Handle case where line_numbers is None instead of an array of line numbers
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: Jonathan Bedard
URL:
Keywords:
Depends on: 173559
Blocks:
  Show dependency treegraph
 
Reported: 2017-07-24 00:37 PDT by Manuel Rego Casasnovas
Modified: 2017-07-24 10:31 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.96 KB, patch)
2017-07-24 08:31 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (2.93 KB, patch)
2017-07-24 09:43 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch for landing (2.95 KB, patch)
2017-07-24 09:53 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.