Bug 161247 - Check-webkit-style does not work with Objective-C blocks
Summary: Check-webkit-style does not work with Objective-C blocks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Enhancement
Assignee: Jonathan Bedard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-26 09:09 PDT by Jonathan Bedard
Modified: 2016-08-29 09:21 PDT (History)
6 users (show)

See Also:


Attachments
Patch (8.73 KB, patch)
2016-08-26 09:16 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (9.38 KB, patch)
2016-08-26 10:04 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (9.77 KB, patch)
2016-08-26 10:41 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 Jonathan Bedard 2016-08-26 09:09:22 PDT
Check-webkit-style erroneously marks properly formatted lambda functions in Objective C and Objective C++ code.
Comment 1 Jonathan Bedard 2016-08-26 09:16:10 PDT
Created attachment 287111 [details]
Patch
Comment 2 Alexey Proskuryakov 2016-08-26 09:18:08 PDT
Comment on attachment 287111 [details]
Patch

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

> Tools/ChangeLog:3
> +        Check-webkit-style does not work with Lambda functions in Objective C

The official name for these is "block".

https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/WorkingwithBlocks/WorkingwithBlocks.html
Comment 3 Jonathan Bedard 2016-08-26 09:19:45 PDT
Comment on attachment 287111 [details]
Patch

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

>> Tools/ChangeLog:3
>> +        Check-webkit-style does not work with Lambda functions in Objective C
> 
> The official name for these is "block".
> 
> https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/WorkingwithBlocks/WorkingwithBlocks.html

Good to know.  This patch needs updating then.
Comment 4 Jonathan Bedard 2016-08-26 10:04:34 PDT
Created attachment 287116 [details]
Patch
Comment 5 Jonathan Bedard 2016-08-26 10:13:22 PDT
This patch makes a best attempt to enforce WebKit style guidelines on Objective C blocks.  The trouble is that our style guidelines do not explicitly cover Objective C blocks, a few examples:

This would seem to be consistent with our handling of spaces before brackets:
    ^ {...
This would seem to be consistent with our handling of function declarations:
    ^(int arg1, int arg2) {...

The above examples follow the rules implemented by this patch, so all of the following are illegal:
    ^{...
    ^ (int arg1, int arg2) {...
    ^(int arg1, int arg2){...

Should we document these style guidelines explicitly, or just allow check-webkit-style to enforce them?
Comment 6 Jonathan Bedard 2016-08-26 10:41:19 PDT
Created attachment 287120 [details]
Patch
Comment 7 Brady Eidson 2016-08-27 22:04:20 PDT
(In reply to comment #5)
> This patch makes a best attempt to enforce WebKit style guidelines on
> Objective C blocks.  The trouble is that our style guidelines do not
> explicitly cover Objective C blocks, a few examples:
> 
> This would seem to be consistent with our handling of spaces before brackets:
>     ^ {...
> This would seem to be consistent with our handling of function declarations:
>     ^(int arg1, int arg2) {...
> 
> The above examples follow the rules implemented by this patch, so all of the
> following are illegal:
>     ^{...
>     ^ (int arg1, int arg2) {...
>     ^(int arg1, int arg2){...
> 
> Should we document these style guidelines explicitly, or just allow
> check-webkit-style to enforce them?

We should document them as well, but doing so should not hold up this patch; It's okay for check-webkit-style to catch them before they're documented.
Comment 8 WebKit Commit Bot 2016-08-29 09:20:54 PDT
Comment on attachment 287120 [details]
Patch

Clearing flags on attachment: 287120

Committed r205122: <http://trac.webkit.org/changeset/205122>
Comment 9 WebKit Commit Bot 2016-08-29 09:21:08 PDT
All reviewed patches have been landed.  Closing bug.