WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
210682
check-webkit-style false positive for [readability/naming/protected] with method declaration
https://bugs.webkit.org/show_bug.cgi?id=210682
Summary
check-webkit-style false positive for [readability/naming/protected] with met...
David Kilzer (:ddkilzer)
Reported
2020-04-17 20:19:46 PDT
check-webkit-style false positive for [readability/naming/protected] with method declaration. Ref<GeolocationPermissionRequestProxy> createRequest(GeolocationIdentifier); ERROR: Source/WebKit/UIProcess/GeolocationPermissionRequestManagerProxy.h:46: 'createRequest' is incorrectly named. It should be named 'protector' or 'protectedGeolocationIdentifier'. [readability/naming/protected] [4]
Attachments
Patch v1
(3.90 KB, patch)
2020-04-17 20:27 PDT
,
David Kilzer (:ddkilzer)
no flags
Details
Formatted Diff
Diff
Patch v2
(3.97 KB, patch)
2020-05-05 17:59 PDT
,
David Kilzer (:ddkilzer)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1
2020-04-17 20:27:54 PDT
Created
attachment 396829
[details]
Patch v1
David Kilzer (:ddkilzer)
Comment 2
2020-04-17 20:43:32 PDT
Comment on
attachment 396829
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=396829&action=review
> Tools/Scripts/webkitpy/style/checkers/cpp.py:3676 > + # Ignore function declarations where cap_protected_name == protected_name indicates a type name. > + if not cap_protected_name == protected_name:
I considered checking whether `protector_name` started with 'create' instead, but I thought the capitalization check would likely result in better results based on our rules for starting variables with lowercase letters.
Jonathan Bedard
Comment 3
2020-04-20 07:42:41 PDT
Comment on
attachment 396829
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=396829&action=review
>> Tools/Scripts/webkitpy/style/checkers/cpp.py:3676 >> + if not cap_protected_name == protected_name: > > I considered checking whether `protector_name` started with 'create' instead, but I thought the capitalization check would likely result in better results based on our rules for starting variables with lowercase letters.
Feel like we would usually use != instead of not *** == ***.
> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:5787 > + self.assert_lint('Ref<GeolocationPermissionRequestProxy> createRequest(GeolocationIdentifier);', '')
Could we have a counter-example? Ie, a line where cap_protected_name == protected_name and we don't lint?
David Kilzer (:ddkilzer)
Comment 4
2020-05-05 16:51:13 PDT
Comment on
attachment 396829
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=396829&action=review
>>> Tools/Scripts/webkitpy/style/checkers/cpp.py:3676 >>> + if not cap_protected_name == protected_name: >> >> I considered checking whether `protector_name` started with 'create' instead, but I thought the capitalization check would likely result in better results based on our rules for starting variables with lowercase letters. > > Feel like we would usually use != instead of not *** == ***.
Will fix.
>> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:5787 >> + self.assert_lint('Ref<GeolocationPermissionRequestProxy> createRequest(GeolocationIdentifier);', '') > > Could we have a counter-example? Ie, a line where cap_protected_name == protected_name and we don't lint?
The counter-example should never exist because we wouldn't name a variable that started with a capital letter!
David Kilzer (:ddkilzer)
Comment 5
2020-05-05 17:57:36 PDT
Comment on
attachment 396829
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=396829&action=review
>>> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:5787 >>> + self.assert_lint('Ref<GeolocationPermissionRequestProxy> createRequest(GeolocationIdentifier);', '') >> >> Could we have a counter-example? Ie, a line where cap_protected_name == protected_name and we don't lint? > > The counter-example should never exist because we wouldn't name a variable that started with a capital letter!
Please disregard this comment. I understand what you meant now. Will add.
David Kilzer (:ddkilzer)
Comment 6
2020-05-05 17:59:38 PDT
Created
attachment 398575
[details]
Patch v2
Darin Adler
Comment 7
2020-05-05 18:17:51 PDT
Comment on
attachment 398575
[details]
Patch v2 I don’t understand the code, but I understand the test cases.
EWS
Comment 8
2020-05-08 14:14:29 PDT
Committed
r261415
: <
https://trac.webkit.org/changeset/261415
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 398575
[details]
.
Radar WebKit Bug Importer
Comment 9
2020-05-08 14:15:20 PDT
<
rdar://problem/63035760
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug