[PATCH] Detect suspicious use of semicolon after 'if/for/while'

statements if they are followed by a {..} block.

Examples are:

for (int i = 0; i < 10; ++i);
{
   printf("i)";
}

or

if (i == 100);
{
   die("Wrong argument");
}

This new check is active if you enable inconclusive checks.
This commit is contained in:
Thomas Jarosch 2011-10-11 08:41:39 +02:00 committed by Daniel Marjamäki
parent fcf360825a
commit 849bee8437
3 changed files with 126 additions and 1 deletions

View File

@ -264,6 +264,39 @@ void CheckOther::bitwiseOnBooleanError(const Token *tok, const std::string &varn
"Boolean variable '" + varname + "' is used in bitwise operation. Did you mean " + op + " ?"); "Boolean variable '" + varname + "' is used in bitwise operation. Did you mean " + op + " ?");
} }
void CheckOther::checkSuspiciousSemicolon()
{
if (!_settings->inconclusive || !_settings->isEnabled("style"))
return;
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
{
// Look for "if(); {}", "for(); {}" or "while(); {}"
if (Token::Match(tok, "if|for|while ("))
{
const Token *end = tok->next()->link();
if (!end)
continue;
// Ensure the semicolon is at the same line number as the if/for/while statement
// and the {..} block follows it without an extra empty line.
if (Token::simpleMatch(end, ") { ; } {") &&
end->linenr() == end->tokAt(2)->linenr()
&& end->linenr()+1 >= end->tokAt(4)->linenr())
{
SuspiciousSemicolonError(tok);
}
}
}
}
void CheckOther::SuspiciousSemicolonError(const Token* tok)
{
reportInconclusiveError(tok, Severity::warning, "suspiciousSemicolon",
"Suspicious use of ; at the end of 'if/for/while' statement.");
}
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckOther::warningOldStylePointerCast() void CheckOther::warningOldStylePointerCast()

View File

@ -66,6 +66,7 @@ public:
checkOther.checkDuplicateBranch(); checkOther.checkDuplicateBranch();
checkOther.checkDuplicateExpression(); checkOther.checkDuplicateExpression();
checkOther.checkDuplicateBreak(); checkOther.checkDuplicateBreak();
checkOther.checkSuspiciousSemicolon();
// information checks // information checks
checkOther.checkVariableScope(); checkOther.checkVariableScope();
@ -233,6 +234,9 @@ public:
/** @brief %Check for comparing a bool expression with an integer other than 0 or 1 */ /** @brief %Check for comparing a bool expression with an integer other than 0 or 1 */
void checkComparisonOfBoolExpressionWithInt(); void checkComparisonOfBoolExpressionWithInt();
/** @brief %Check for suspicious use of semicolon */
void checkSuspiciousSemicolon();
// Error messages.. // Error messages..
void cstyleCastError(const Token *tok); void cstyleCastError(const Token *tok);
void dangerousUsageStrtolError(const Token *tok); void dangerousUsageStrtolError(const Token *tok);
@ -271,6 +275,7 @@ public:
void unsignedPositiveError(const Token *tok, const std::string &varname); void unsignedPositiveError(const Token *tok, const std::string &varname);
void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op); void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op);
void comparisonOfBoolExpressionWithIntError(const Token *tok); void comparisonOfBoolExpressionWithIntError(const Token *tok);
void SuspiciousSemicolonError(const Token *tok);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)
{ {
@ -321,6 +326,7 @@ public:
c.unsignedPositiveError(0, "varname"); c.unsignedPositiveError(0, "varname");
c.bitwiseOnBooleanError(0, "varname", "&&"); c.bitwiseOnBooleanError(0, "varname", "&&");
c.comparisonOfBoolExpressionWithIntError(0); c.comparisonOfBoolExpressionWithIntError(0);
c.SuspiciousSemicolonError(0);
} }
std::string myName() const std::string myName() const
@ -371,6 +377,7 @@ public:
"* testing if unsigned variable is negative\n" "* testing if unsigned variable is negative\n"
"* testing is unsigned variable is positive\n" "* testing is unsigned variable is positive\n"
"* using bool in bitwise expression\n" "* using bool in bitwise expression\n"
"* Suspicious use of ; at the end of 'if/for/while' statement.\n"
// optimisations // optimisations
"* optimisation: detect post increment/decrement\n"; "* optimisation: detect post increment/decrement\n";

View File

@ -140,6 +140,9 @@ private:
TEST_CASE(alwaysTrueFalseStringCompare); TEST_CASE(alwaysTrueFalseStringCompare);
TEST_CASE(checkSignOfUnsignedVariable); TEST_CASE(checkSignOfUnsignedVariable);
TEST_CASE(checkForSuspiciousSemicolon1);
TEST_CASE(checkForSuspiciousSemicolon2);
} }
void check(const char code[], const char *filename = NULL) void check(const char code[], const char *filename = NULL)
@ -170,6 +173,7 @@ private:
checkOther.checkDuplicateExpression(); checkOther.checkDuplicateExpression();
checkOther.checkBitwiseOnBoolean(); checkOther.checkBitwiseOnBoolean();
checkOther.checkComparisonOfBoolExpressionWithInt(); checkOther.checkComparisonOfBoolExpressionWithInt();
checkOther.checkSuspiciousSemicolon();
// Simplify token list.. // Simplify token list..
tokenizer.simplifyTokenList(); tokenizer.simplifyTokenList();
@ -3645,7 +3649,88 @@ private:
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void checkForSuspiciousSemicolon1()
{
check(
"void foo() {\n"
" for(int i = 0; i < 10; ++i);\n"
"}");
ASSERT_EQUALS("", errout.str());
// Empty block
check(
"void foo() {\n"
" for(int i = 0; i < 10; ++i); {\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious use of ; at the end of 'if/for/while' statement.\n", errout.str());
// Block with some tokens to make sure the tokenizer output
// stays the same for "for(); {}"
check(
"void foo() {\n"
" for(int i = 0; i < 10; ++i); {\n"
" int j = 123;\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious use of ; at the end of 'if/for/while' statement.\n", errout.str());
check(
"void foo() {\n"
" while (!quit); {\n"
" do_something();\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious use of ; at the end of 'if/for/while' statement.\n", errout.str());
}
void checkForSuspiciousSemicolon2()
{
check(
"void foo() {\n"
" if (i == 1); {\n"
" do_something();\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious use of ; at the end of 'if/for/while' statement.\n", errout.str());
// Seen this in the wild
check(
"void foo() {\n"
" if (Match());\n"
" do_something();\n"
"}");
ASSERT_EQUALS("", errout.str());
check(
"void foo() {\n"
" if (Match());\n"
" else\n"
" do_something();\n"
"}");
ASSERT_EQUALS("", errout.str());
check(
"void foo() {\n"
" if (i == 1)\n"
" ;\n"
" {\n"
" do_something();\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
check(
"void foo() {\n"
" if (i == 1);\n"
"\n"
" {\n"
" do_something();\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
}
}; };
REGISTER_TEST(TestOther) REGISTER_TEST(TestOther)