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
Patch v2 (3.97 KB, patch)
2020-05-05 17:59 PDT, David Kilzer (:ddkilzer)
no flags
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
Note You need to log in before you can comment on or make changes to this bug.