Bug 51303 - check-webkit-style should detect function declarations (and trivial functions).
Summary: check-webkit-style should detect function declarations (and trivial functions).
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: David Levin
URL:
Keywords:
Depends on:
Blocks: 49394
  Show dependency treegraph
 
Reported: 2010-12-18 23:06 PST by David Levin
Modified: 2010-12-20 11:39 PST (History)
1 user (show)

See Also:


Attachments
Patch (8.84 KB, patch)
2010-12-18 23:49 PST, David Levin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Levin 2010-12-18 23:06:02 PST
This is needed for bug 49394 (which examines declarations).
Comment 1 David Levin 2010-12-18 23:49:10 PST
Created attachment 76955 [details]
Patch
Comment 2 Shinichiro Hamaji 2010-12-19 12:32:14 PST
Comment on attachment 76955 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=76955&action=review

Looks good, putting some nitpicks on test. Please feel free to ignore some of (or even all of) them if they don't make sense.

> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:240
> +        if not function_information:

I'd add a trivial test case which passes this case. Not sure, but "if (foobar) { something... }" or "for (foobar;...) { ... }" would be good candidates?

> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:2818
> +        self.assert_pass_ptr_check(

s/pass_ptr/pass_ref_ptr/ ?

> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:2823
> +        self.assert_pass_ptr_check(

s/pass_ptr/pass_ref_ptr/ ?

> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:2837
> +            '    PassRefPtr<Type1> m_other;\n'

I guess RefPtr would be more realistic?
Comment 3 David Levin 2010-12-20 11:39:01 PST
(In reply to comment #2)
> (From update of attachment 76955 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=76955&action=review
> 
> Looks good, putting some nitpicks on test. Please feel free to ignore some of (or even all of) them if they don't make sense.
> 
> > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:240
> > +        if not function_information:
> 
> I'd add a trivial test case which passes this case. Not sure, but "if (foobar) { something... }" or "for (foobar;...) { ... }" would be good candidates?

Done.

> > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:2818
> > +        self.assert_pass_ptr_check(
> 
> s/pass_ptr/pass_ref_ptr/ ?

I left this as is because the test is about Pass*Ptr. (It checks Pass.*Ptr so that it catches PassRefPtr and PassOwnPtr. Other things named the same way should follow the same rule.)


> > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:2837
> > +            '    PassRefPtr<Type1> m_other;\n'
> 
> I guess RefPtr would be more realistic?

Done. (Good point :).)
Comment 4 David Levin 2010-12-20 11:39:38 PST
Committed as http://trac.webkit.org/changeset/74356