Bug 219092

Summary: check-webkit-style should require spaces around the equal sign for Objective-C @synthesize
Product: WebKit Reporter: Hoa Dinh <dvh>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: ews-watchlist, glenn, jbedard, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Other   
OS: Other   
Bug Depends on: 219094    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Hoa Dinh 2020-11-18 09:02:06 PST
Currently, check-webkit-style will require to have no spaces around the equal sign for "@synthesize a = b".
The codebase shows that "@synthesize a=b" is preferred.

Let's improve check-webkit-style to match what we use.
Comment 1 Hoa Dinh 2020-11-18 09:07:42 PST
Created attachment 414455 [details]
Patch
Comment 2 Jonathan Bedard 2020-11-18 09:18:23 PST
Comment on attachment 414455 [details]
Patch

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

> Tools/ChangeLog:3
> +        check-webkit-style: requires spaces around the equal sign for

Should be on a single line

> Tools/Scripts/webkitpy/style/checkers/cpp.py:2208
> +                  'Should have spaces around = in property synthesis.')

I don't have an issue with the change, but I also don't do much workin Objective-C....someone who has more experience in the area should verify this is actually what we want to do.
Comment 3 Wenson Hsieh 2020-11-18 09:45:03 PST
Comment on attachment 414455 [details]
Patch

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

> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:2109
> +        self.assert_lint('@synthesize a=b;', 'Should have spaces around = in property synthesis.  [whitespace/property] [4]')

It looks like there are currently about 90 instances of `@synthesize foo=bar` in the WebKit stack — we should probably update these if we're changing the style rules for synthesized properties.
Comment 4 Wenson Hsieh 2020-11-18 10:06:09 PST
(In reply to Wenson Hsieh from comment #3)
> Comment on attachment 414455 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414455&action=review
> 
> > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:2109
> > +        self.assert_lint('@synthesize a=b;', 'Should have spaces around = in property synthesis.  [whitespace/property] [4]')
> 
> It looks like there are currently about 90 instances of `@synthesize
> foo=bar` in the WebKit stack — we should probably update these if we're
> changing the style rules for synthesized properties.

(...but I suppose this could be done separately)
Comment 5 Hoa Dinh 2020-11-18 10:18:21 PST
Created attachment 414458 [details]
Patch
Comment 6 Wenson Hsieh 2020-11-19 09:02:34 PST
Comment on attachment 414458 [details]
Patch

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

> Tools/ChangeLog:6
> +        Reviewed by NOBODY (OOPS!).

Ideally, there should be a sentence or two below the "Reviewed by" line describing the change. Maybe something like along the lines of:

```
Teach the Objective-C style checker to prefer `@synthesize a = b` over `@synthesize a=b`. As a followup,
<webkit.org/b/219094> will enforce this style rule in existing @synthesize statements in WebKit.
```
Comment 7 Hoa Dinh 2020-11-19 11:19:04 PST
Created attachment 414603 [details]
Patch
Comment 8 EWS 2020-11-19 21:49:04 PST
Committed r270068: <https://trac.webkit.org/changeset/270068>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414603 [details].
Comment 9 Radar WebKit Bug Importer 2020-11-19 21:50:20 PST
<rdar://problem/71615614>