Bug 78140 – Add toText and isTextNode helpers in Text class

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Abhishek Arya]

in WebKit/Source/WebCore/dom/Text.h,
toText is needed to shorten static_cast<Text>* and also detect bad cast using isTextNode

[Joe Thomas]

Created attachment 126623 [details]
ProposedPatch

[Adam Barth]

Comment on attachment 126623 [details]
ProposedPatch

Consider adding a style checker rule to remind folks to use toText rather than static_cast<Text*>

[Joe Thomas]

(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. :(

[Joe Thomas]

(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.

[Joe Thomas]

Created attachment 126670 [details]
Patch-with-StyleChecker

Added style checker.

[WebKit Review Bot]

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.

[Joe Thomas]

(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.

[Adam Barth]

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.

[WebKit Review Bot]

Comment on attachment 126670 [details]
Patch-with-StyleChecker

Clearing flags on attachment: 126670

Committed r107509: <http://trac.webkit.org/changeset/107509>

[David Levin]

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!

[Joe Thomas]

(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?

[David Levin]

(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

[Joe Thomas]

Created attachment 126692 [details]
UnitTestCase-StyleChecker

Added the unit test case for the style checker introduced in the previous commit for this bug

[Adam Barth]

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.

[WebKit Review Bot]

Comment on attachment 126692 [details]
UnitTestCase-StyleChecker

Clearing flags on attachment: 126692

Committed r107522: <http://trac.webkit.org/changeset/107522>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.