Bug 27498 - Feature request: cpplint should check for braces - rule 3
Summary: Feature request: cpplint should check for braces - rule 3
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-21 07:30 PDT by Luciano Wolf
Modified: 2009-07-21 15:48 PDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Luciano Wolf 2009-07-21 07:30:52 PDT
According to Braces rule 3 of the Webkit coding style guidelines[1]:
"One-line control clauses should not use braces unless ... a single statement spans multiple lines."

cpplint doesn't complain about non-compliance with this rule when using the Qt foreach macro, as illustrated below

WRONG CODING STYLE: 

    foreach (AbstractMetaFunction *func, lst)
        qDebug() << "*" << func->ownerClass()->name()
                        << "Private: " << func->isPrivate()
                        << "Empty: " << func->isEmptyFunction();

RIGHT CODING STYLE:

    foreach (AbstractMetaFunction *func, lst) {
        qDebug() << "*" << func->ownerClass()->name()
                        << "Private: " << func->isPrivate()
                        << "Empty: " << func->isEmptyFunction();
    }

[1]
http://webkit.org/coding/coding-style.html
Comment 1 Kenneth Rohde Christiansen 2009-07-21 15:33:42 PDT
Actually I believe that we still have no code checking for "missing" braces. And this is basically not Qt specific, but a case of a single statement spanning multiply lines.

Actually I just did the test:

3096         self.assert_multi_line_lint(
3097             'if (true)\n'
3098             '    myFunction(reallyLongParam1, reallyLongParam2,\n'
3099             '               reallyLongParam3);\n'
3100             '\n',
3101             '')

and it didn't catch the error, this should be using { }.
Comment 2 David Levin 2009-07-21 15:37:24 PDT
Yes the code was added for telling you when to not have them, but there was no coded added for telling you when to have them.
Comment 3 Adam Treat 2009-07-21 15:45:09 PDT
Lu(In reply to comment #1)
> Actually I believe that we still have no code checking for "missing" braces.
> And this is basically not Qt specific, but a case of a single statement
> spanning multiply lines.
> 
> Actually I just did the test:
> 
> 3096         self.assert_multi_line_lint(
> 3097             'if (true)\n'
> 3098             '    myFunction(reallyLongParam1, reallyLongParam2,\n'
> 3099             '               reallyLongParam3);\n'
> 3100             '\n',
> 3101             '')
> 
> and it didn't catch the error, this should be using { }.

It is a good thing we have a little tool called a *compiler* to tell us that :)

wink wink
nudge nudge
Comment 4 Kenneth Rohde Christiansen 2009-07-21 15:47:12 PDT
Notice, it is a SINGLE statement, spanning MULTIPLY lines :-)
Comment 5 Adam Treat 2009-07-21 15:48:36 PDT
(In reply to comment #4)
> Notice, it is a SINGLE statement, spanning MULTIPLY lines :-)

Oopsie, i read it wrong.  I see what you mean now.