Bug 115567 – check-webkit-style should complain about a layering violation if platform-specific guards are used in WebCore outside of the platform directory

• Component: Tools / Tests
• Status: CLOSED WONTFIX

[Jessie Berlin]

This should help in discouraging folks from adding more layering violations between WebCore and WebCore/platform.

[Jessie Berlin]

Created attachment 200459 [details]
Patch

[Benjamin Poulain]

Comment on attachment 200459 [details]
Patch

Awesome idea!

I would have a couple more tests for:
-various spacing
-ifdef in a comment.

[Jessie Berlin]

(In reply to comment #2)
> (From update of attachment 200459 [details])
> Awesome idea!
> 
> I would have a couple more tests for:
> -various spacing

I added

        error_collector = ErrorCollector(self.assertTrue)
        self.process_file_data('Source/WebCore/loader/NavigationAction.cpp', 'cpp', ['#if PLATFORM ( MAC )', '#endif'], error_collector)
        self.assertEqual(1, error_collector.result_list().count(errmsg))

> -ifdef in a comment.

Not 100% sure what you mean by this. Do you mean

// #if PLATFORM(MAC)

?

And would you expect that to cause the style error to be thrown? I don’t think I would.

[Jessie Berlin]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 200459 [details] [details])
> > Awesome idea!
> > 
> > I would have a couple more tests for:
> > -various spacing
> 
> I added
> 
>         error_collector = ErrorCollector(self.assertTrue)
>         self.process_file_data('Source/WebCore/loader/NavigationAction.cpp', 'cpp', ['#if PLATFORM ( MAC )', '#endif'], error_collector)
>         self.assertEqual(1, error_collector.result_list().count(errmsg))
> 
> > -ifdef in a comment.
> 
> Not 100% sure what you mean by this. Do you mean
> 
> // #if PLATFORM(MAC)
> 
> ?
> 
> And would you expect that to cause the style error to be thrown? I don’t think I would.

I double checked with Ben and I will add this test with the expectation that the style error will not be thrown in this case.

[Jessie Berlin]

Comment on attachment 200459 [details]
Patch

Committed in http://trac.webkit.org/changeset/149635

[Csaba Osztrogonác]

(In reply to comment #5)
> (From update of attachment 200459 [details])
> Committed in http://trac.webkit.org/changeset/149635

The new webkitpy test fails on the bots, fix is coming soon 
in https://bugs.webkit.org/show_bug.cgi?id=116016

[Jessie Berlin]

Rolled out in http://trac.webkit.org/changeset/150457, it doesn’t make sense to do this at this time.