in WebKit/Source/WebCore/dom/Text.h, toText is needed to shorten static_cast<Text>* and also detect bad cast using isTextNode
Created attachment 126623 [details] ProposedPatch
Comment on attachment 126623 [details] ProposedPatch Consider adding a style checker rule to remind folks to use toText rather than static_cast<Text*>
(In reply to comment #2) > (From update of attachment 126623 [details]) > Consider adding a style checker rule to remind folks to use toText rather than static_cast<Text*> Checked Tools/Scripts/webkitpy/style/, but did not get much idea how to do this. :(
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 126623 [details] [details]) > > Consider adding a style checker rule to remind folks to use toText rather than static_cast<Text*> > > Checked Tools/Scripts/webkitpy/style/, but did not get much idea how to do this. :( I was trying to report the style check error with one of the error category mentioned in BASE_FILTER_RULES in Tools/Scripts/webkitpy/style/checker.py. As far as I understand, errors reported with this list is ignored.
Created attachment 126670 [details] Patch-with-StyleChecker Added style checker.
Attachment 126670 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/dom/Text.h:76: Consider using toText helper function in WebCore/dom/Text.h instead of static_cast<Text*> [readability/check] [4] Total errors found: 1 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #6) > Attachment 126670 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 > > Source/WebCore/dom/Text.h:76: Consider using toText helper function in WebCore/dom/Text.h instead of static_cast<Text*> [readability/check] [4] > Total errors found: 1 in 37 files This error is expected as it is caused by the new style-checker added in this patch.
Comment on attachment 126670 [details] Patch-with-StyleChecker Oh, I meant to make the style checker change in a separate patch, but this seems fine too.
Comment on attachment 126670 [details] Patch-with-StyleChecker Clearing flags on attachment: 126670 Committed r107509: <http://trac.webkit.org/changeset/107509>
It looks like there wasn't a unit test for the style checker change. Please add one -- otherwise it may break at anytime due to people changing the code and no one will know!
(In reply to comment #10) > It looks like there wasn't a unit test for the style checker change. > > Please add one -- otherwise it may break at anytime due to people changing the code and no one will know! Sure I will add the unit test case. I did not know that there is a unit test suite for style checker. What is the script used for running the unit test?
(In reply to comment #11) > (In reply to comment #10) > > It looks like there wasn't a unit test for the style checker change. > > > > Please add one -- otherwise it may break at anytime due to people changing the code and no one will know! > > Sure I will add the unit test case. I did not know that there is a unit test suite for style checker. What is the script used for running the unit test? Here's a sample change: http://trac.webkit.org/changeset/105652
Created attachment 126692 [details] UnitTestCase-StyleChecker Added the unit test case for the style checker introduced in the previous commit for this bug
Comment on attachment 126692 [details] UnitTestCase-StyleChecker View in context: https://bugs.webkit.org/attachment.cgi?id=126692&action=review > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:762 > + # Test for static_cast readability. This comment doesn't seem to be adding any value. It's redundant with the function name.
Comment on attachment 126692 [details] UnitTestCase-StyleChecker Clearing flags on attachment: 126692 Committed r107522: <http://trac.webkit.org/changeset/107522>
All reviewed patches have been landed. Closing bug.