diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 791954e98..63f3c5968 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -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 + " ?"); } +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() diff --git a/lib/checkother.h b/lib/checkother.h index 47c9e940a..5aca5ccbb 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -66,6 +66,7 @@ public: checkOther.checkDuplicateBranch(); checkOther.checkDuplicateExpression(); checkOther.checkDuplicateBreak(); + checkOther.checkSuspiciousSemicolon(); // information checks checkOther.checkVariableScope(); @@ -233,6 +234,9 @@ public: /** @brief %Check for comparing a bool expression with an integer other than 0 or 1 */ void checkComparisonOfBoolExpressionWithInt(); + /** @brief %Check for suspicious use of semicolon */ + void checkSuspiciousSemicolon(); + // Error messages.. void cstyleCastError(const Token *tok); void dangerousUsageStrtolError(const Token *tok); @@ -271,6 +275,7 @@ public: void unsignedPositiveError(const Token *tok, const std::string &varname); void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op); void comparisonOfBoolExpressionWithIntError(const Token *tok); + void SuspiciousSemicolonError(const Token *tok); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { @@ -321,6 +326,7 @@ public: c.unsignedPositiveError(0, "varname"); c.bitwiseOnBooleanError(0, "varname", "&&"); c.comparisonOfBoolExpressionWithIntError(0); + c.SuspiciousSemicolonError(0); } std::string myName() const @@ -371,6 +377,7 @@ public: "* testing if unsigned variable is negative\n" "* testing is unsigned variable is positive\n" "* using bool in bitwise expression\n" + "* Suspicious use of ; at the end of 'if/for/while' statement.\n" // optimisations "* optimisation: detect post increment/decrement\n"; diff --git a/test/testother.cpp b/test/testother.cpp index 1c8eb7616..b5cac3077 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -140,6 +140,9 @@ private: TEST_CASE(alwaysTrueFalseStringCompare); TEST_CASE(checkSignOfUnsignedVariable); + + TEST_CASE(checkForSuspiciousSemicolon1); + TEST_CASE(checkForSuspiciousSemicolon2); } void check(const char code[], const char *filename = NULL) @@ -170,6 +173,7 @@ private: checkOther.checkDuplicateExpression(); checkOther.checkBitwiseOnBoolean(); checkOther.checkComparisonOfBoolExpressionWithInt(); + checkOther.checkSuspiciousSemicolon(); // Simplify token list.. tokenizer.simplifyTokenList(); @@ -3645,7 +3649,88 @@ private: "}"); 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) -