Bug 27498
| Summary: | Feature request: cpplint should check for braces - rule 3 | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Luciano Wolf <luciano.wolf> |
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Minor | CC: | kenneth, levin, manyoso |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | All | ||
Luciano Wolf
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
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Kenneth Rohde Christiansen
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 { }.
David Levin
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.
Adam Treat
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
Kenneth Rohde Christiansen
Notice, it is a SINGLE statement, spanning MULTIPLY lines :-)
Adam Treat
(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.