From 93ea774484498d4fd2b00634873ec1f190438d83 Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Sat, 19 Feb 2011 21:33:29 +1300 Subject: [PATCH] initial simplistic implementation of switchCaseFallThrough --- lib/checkother.cpp | 46 +++++++++++++++++++ lib/checkother.h | 6 +++ lib/preprocessor.cpp | 7 ++- test/testother.cpp | 106 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 8588e3666..b7e3e6e19 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -306,6 +306,46 @@ void CheckOther::checkRedundantAssignmentInSwitch() } +void CheckOther::checkSwitchCaseFallThrough() +{ + const char switchPattern[] = "switch ( %any% ) { case"; + //const char breakPattern[] = "break|continue|return|exit|goto"; + //const char functionPattern[] = "%var% ("; + // nested switch + + // Find the beginning of a switch. E.g.: + // switch (var) { ... + const Token *tok = Token::findmatch(_tokenizer->tokens(), switchPattern); + while (tok) + { + + // Check the contents of the switch statement + for (const Token *tok2 = tok->tokAt(6); tok2; tok2 = tok2->next()) + { + if (Token::Match(tok2, switchPattern)) + { + tok2 = tok2->tokAt(4)->link()->previous(); + } + else if (tok2->str() == "case") + { + if (!Token::Match(tok2->previous()->previous(), "break")) + { + switchCaseFallThrough(tok2); + } + } + else if (tok2->str() == "}") + { + // End of the switch block + break; + } + + } + + tok = Token::findmatch(tok->next(), switchPattern); + } +} + + //--------------------------------------------------------------------------- // int x = 1; // x = x; // <- redundant assignment to self @@ -2998,6 +3038,12 @@ void CheckOther::redundantAssignmentInSwitchError(const Token *tok, const std::s "redundantAssignInSwitch", "Redundant assignment of \"" + varname + "\" in switch"); } +void CheckOther::switchCaseFallThrough(const Token *tok) +{ + reportError(tok, Severity::warning, + "switchCaseFallThrough", "Switch falls through case without comment"); +} + void CheckOther::selfAssignmentError(const Token *tok, const std::string &varname) { reportError(tok, Severity::warning, diff --git a/lib/checkother.h b/lib/checkother.h index d43c4c413..63c7ebed7 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -61,6 +61,7 @@ public: checkOther.sizeofsizeof(); checkOther.sizeofCalculation(); checkOther.checkRedundantAssignmentInSwitch(); + checkOther.checkSwitchCaseFallThrough(); checkOther.checkAssignmentInAssert(); checkOther.checkSizeofForArrayParameter(); checkOther.checkSelfAssignment(); @@ -162,6 +163,9 @@ public: /** @brief %Check for assigning to the same variable twice in a switch statement*/ void checkRedundantAssignmentInSwitch(); + /** @brief %Check for switch case fall through without comment */ + void checkSwitchCaseFallThrough(); + /** @brief %Check for assigning a variable to itself*/ void checkSelfAssignment(); @@ -209,6 +213,7 @@ public: void mathfunctionCallError(const Token *tok, const unsigned int numParam = 1); void fflushOnInputStreamError(const Token *tok, const std::string &varname); void redundantAssignmentInSwitchError(const Token *tok, const std::string &varname); + void switchCaseFallThrough(const Token *tok); void selfAssignmentError(const Token *tok, const std::string &varname); void assignmentInAssertError(const Token *tok, const std::string &varname); void incorrectLogicOperatorError(const Token *tok); @@ -247,6 +252,7 @@ public: c.sizeofsizeofError(0); c.sizeofCalculationError(0); c.redundantAssignmentInSwitchError(0, "varname"); + c.switchCaseFallThrough(0); c.selfAssignmentError(0, "varname"); c.assignmentInAssertError(0, "varname"); c.invalidScanfError(0); diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 45ed8a425..9f4bb0384 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -377,10 +377,10 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri i = str.find('\n', i); if (i == std::string::npos) break; + std::string comment(str, commentStart, i - commentStart); if (settings && settings->_inlineSuppressions) { - std::string comment(str, commentStart, i - commentStart); std::istringstream iss(comment); std::string word; iss >> word; @@ -392,6 +392,11 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri } } + if (comment.find("fall through") != std::string::npos) + { + suppressionIDs.push_back("switchCaseFallThrough"); + } + code << "\n"; previous = '\n'; ++lineno; diff --git a/test/testother.cpp b/test/testother.cpp index 5707fc264..43d07ee8b 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +#include "preprocessor.h" #include "tokenize.h" #include "checkother.h" #include "testsuite.h" @@ -74,6 +75,7 @@ private: TEST_CASE(sizeofCalculation); TEST_CASE(switchRedundantAssignmentTest); + TEST_CASE(switchFallThroughCase); TEST_CASE(selfAssignment); TEST_CASE(testScanf1); @@ -148,6 +150,60 @@ private: checkOther.checkComparisonOfBoolWithInt(); } + class SimpleSuppressor: public ErrorLogger + { + public: + SimpleSuppressor(Settings &settings, ErrorLogger *next) + : _settings(settings), _next(next) + { } + virtual void reportOut(const std::string &outmsg) + { + _next->reportOut(outmsg); + } + virtual void reportErr(const ErrorLogger::ErrorMessage &msg) + { + if (!msg._callStack.empty() && !_settings.nomsg.isSuppressed(msg._id, msg._callStack.begin()->getfile(), msg._callStack.begin()->line)) + _next->reportErr(msg); + } + virtual void reportStatus(unsigned int index, unsigned int max) + { + _next->reportStatus(index, max); + } + private: + Settings &_settings; + ErrorLogger *_next; + }; + + void check_preprocess_suppress(const char precode[], const char *filename = NULL) + { + // Clear the error buffer.. + errout.str(""); + + if (filename == NULL) + filename = "test.cpp"; + + Settings settings; + settings._checkCodingStyle = true; + + // Preprocess file.. + Preprocessor preprocessor(&settings, this); + std::list configurations; + std::string filedata = ""; + std::istringstream fin(precode); + preprocessor.preprocess(fin, filedata, configurations, filename, settings._includePaths); + SimpleSuppressor logger(settings, this); + const std::string code = Preprocessor::getcode(filedata, "", filename, &settings, &logger); + + // Tokenize.. + Tokenizer tokenizer(&settings, &logger); + std::istringstream istr(code); + tokenizer.tokenize(istr, filename); + + // Check.. + CheckOther checkOther(&tokenizer, &settings, &logger); + checkOther.checkSwitchCaseFallThrough(); + } + void zeroDiv1() { @@ -1146,6 +1202,56 @@ private: ASSERT_EQUALS("", errout.str()); } + void switchFallThroughCase() + { + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " break;\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (warning) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " // fall through\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " break;\n" + " // fall through\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void selfAssignment() { check("void foo()\n"