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]
Created attachment 396829 [details] Patch v1
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.
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?
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!
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.
Created attachment 398575 [details] Patch v2
Comment on attachment 398575 [details] Patch v2 I don’t understand the code, but I understand the test cases.
Committed r261415: <https://trac.webkit.org/changeset/261415> All reviewed patches have been landed. Closing bug and clearing flags on attachment 398575 [details].
<rdar://problem/63035760>