From bb52a63c4e0481f5693d83770d29088cef70ea2a Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Wed, 24 Jul 2019 09:59:01 +0200 Subject: [PATCH] Add check for const variables When a local reference is declared, this will check if that local reference can be declared as `const`. --- lib/astutils.cpp | 120 +++++++++++-- lib/astutils.h | 6 + lib/checkother.cpp | 83 +++++++++ lib/checkother.h | 8 +- lib/symboldatabase.cpp | 11 +- lib/symboldatabase.h | 1 + test/testcondition.cpp | 208 +++++++++++++++++++++++ test/testother.cpp | 370 ++++++++++++++++++++++++++++++++++++++++- 8 files changed, 780 insertions(+), 27 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index b0c2ae2b7..b97da1069 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -176,6 +176,17 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp, return ret; } +bool isFunctionCall(const Token* tok) +{ + if (Token::Match(tok, "%name% (")) + return true; + if (Token::Match(tok, "%name% <") && Token::simpleMatch(tok->next()->link(), "> (")) + return true; + if (Token::Match(tok, "%name% ::")) + return isFunctionCall(tok->tokAt(2)); + return false; +} + static bool hasToken(const Token * startTok, const Token * stopTok, const Token * tok) { for (const Token * tok2 = startTok; tok2 != stopTok; tok2 = tok2->next()) { @@ -227,8 +238,10 @@ bool precedes(const Token * tok1, const Token * tok2) return tok1->index() < tok2->index(); } -static bool isAliased(const Token * startTok, const Token * endTok, nonneg int varid) +bool isAliased(const Token *startTok, const Token *endTok, nonneg int varid) { + if (!precedes(startTok, endTok)) + return false; for (const Token *tok = startTok; tok != endTok; tok = tok->next()) { if (Token::Match(tok, "= & %varid% ;", varid)) return true; @@ -248,6 +261,18 @@ static bool isAliased(const Token * startTok, const Token * endTok, nonneg int v return false; } +bool isAliased(const Variable *var) +{ + if (!var) + return false; + if (!var->scope()) + return false; + const Token *start = var->declEndToken(); + if (!start) + return false; + return isAliased(start, var->scope()->bodyEnd, var->declarationId()); +} + static bool exprDependsOnThis(const Token *expr, nonneg int depth) { if (!expr) @@ -814,6 +839,19 @@ bool isVariableChangedByFunctionCall(const Token *tok, nonneg int varid, const S isVariableChangedByFunctionCall(tok->astOperand2(), varid, settings, inconclusive); } +static bool isScopeBracket(const Token *tok) +{ + if (!Token::Match(tok, "{|}")) + return false; + if (!tok->scope()) + return false; + if (tok->str() == "{") + return tok->scope()->bodyStart == tok; + if (tok->str() == "}") + return tok->scope()->bodyEnd == tok; + return false; +} + bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, bool *inconclusive) { if (!tok) @@ -832,7 +870,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, parent = parent->astParent(); // passing variable to subfunction? - if (Token::Match(parent, "[(,]")) + if (Token::Match(parent, "[(,{]")) ; else if (Token::simpleMatch(parent, ":")) { while (Token::Match(parent, "[?:]")) @@ -847,24 +885,24 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, // goto start of function call and get argnr int argnr = 0; - while (tok && !Token::Match(tok, "[;{}]")) { + while (tok && !Token::simpleMatch(tok, ";") && !isScopeBracket(tok)) { if (tok->str() == ",") ++argnr; else if (tok->str() == ")") tok = tok->link(); - else if (Token::Match(tok->previous(), "%name% (")) + else if (Token::Match(tok->previous(), "%name% (|{")) break; - else if (Token::simpleMatch(tok->previous(), "> (") && tok->previous()->link()) + else if (Token::Match(tok->previous(), "> (|{") && tok->previous()->link()) break; tok = tok->previous(); } - if (!tok || tok->str() != "(") + if (!Token::Match(tok, "{|(")) return false; - const bool possiblyPassedByReference = (tok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)]")); + const bool possiblyPassedByReference = (tok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]")); tok = tok->previous(); if (tok && tok->link() && tok->str() == ">") tok = tok->link()->previous(); - if (!Token::Match(tok, "%name% [(<]")) + if (!Token::Match(tok, "%name% [({<]")) return false; // not a function => variable not changed // Constructor call @@ -932,6 +970,8 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp) { + if (!precedes(start, end)) + return false; for (const Token *tok = start; tok != end; tok = tok->next()) { if (tok->varId() != varid) { if (globalvar && Token::Match(tok, "%name% (")) @@ -941,20 +981,32 @@ bool isVariableChanged(const Token *start, const Token *end, const nonneg int va } const Token *tok2 = tok; - while (Token::simpleMatch(tok2->astParent(), "*")) + while (Token::simpleMatch(tok2->astParent(), "*") || (Token::simpleMatch(tok2->astParent(), ".") && !Token::simpleMatch(tok2->astParent()->astParent(), "(")) || + (Token::simpleMatch(tok2->astParent(), "[") && tok2 == tok2->astParent()->astOperand1())) tok2 = tok2->astParent(); if (Token::Match(tok2->astParent(), "++|--")) return true; - if (tok2->astParent() && tok2->astParent()->isAssignmentOp() && tok2 == tok2->astParent()->astOperand1()) - return true; + if (tok2->astParent() && tok2->astParent()->isAssignmentOp()) { + if (tok2 == tok2->astParent()->astOperand1()) + return true; + // Check if assigning to a non-const lvalue + const Variable * var = getLHSVariable(tok2->astParent()); + if (var && var->isReference() && !var->isConst() && var->nameToken() && var->nameToken()->next() == tok2->astParent()) { + if (!var->isLocal() || isVariableChanged(var, settings, cpp)) + return true; + } + } if (isLikelyStreamRead(cpp, tok->previous())) return true; + if (isLikelyStream(cpp, tok2)) + return true; + // Member function call - if (Token::Match(tok, "%name% . %name% (")) { + if (tok->variable() && Token::Match(tok2->astParent(), ". %name%") && isFunctionCall(tok2->astParent()->next()) && tok2->astParent()->astOperand1() == tok2) { const Variable * var = tok->variable(); bool isConst = var && var->isConst(); if (!isConst && var) { @@ -966,25 +1018,42 @@ bool isVariableChanged(const Token *start, const Token *end, const nonneg int va const Function * fun = ftok->function(); if (!isConst && (!fun || !fun->isConst())) return true; + else + continue; } - const Token *ftok = tok; - while (ftok && (!Token::Match(ftok, "[({[]") || ftok->isCast())) + const Token *ftok = tok2; + while (ftok && (!Token::Match(ftok, "[({]") || ftok->isCast())) ftok = ftok->astParent(); - if (ftok && Token::Match(ftok->link(), ") !!{")) { + if (ftok && Token::Match(ftok->link(), ")|} !!{")) { + const Token * ptok = tok2; + while (Token::Match(ptok->astParent(), ".|::|[")) + ptok = ptok->astParent(); bool inconclusive = false; - bool isChanged = isVariableChangedByFunctionCall(tok, settings, &inconclusive); + bool isChanged = isVariableChangedByFunctionCall(ptok, settings, &inconclusive); isChanged |= inconclusive; if (isChanged) return true; } - const Token *parent = tok->astParent(); + const Token *parent = tok2->astParent(); while (Token::Match(parent, ".|::")) parent = parent->astParent(); if (parent && parent->tokType() == Token::eIncDecOp) return true; + + if (Token::simpleMatch(tok2->astParent(), ":") && tok2->astParent()->astParent() && Token::simpleMatch(tok2->astParent()->astParent()->previous(), "for (")) { + const Token * varTok = tok2->astParent()->previous(); + if (!varTok) + continue; + const Variable * loopVar = varTok->variable(); + if (!loopVar) + continue; + if (!loopVar->isConst() && loopVar->isReference() && isVariableChanged(loopVar, settings, cpp)) + return true; + continue; + } } return false; } @@ -1072,6 +1141,23 @@ const Token *findLambdaEndToken(const Token *first) return nullptr; } +bool isLikelyStream(bool cpp, const Token *stream) +{ + if (!cpp) + return false; + + if (!stream) + return false; + + if (!Token::Match(stream->astParent(), "&|<<|>>") || !stream->astParent()->isBinaryOp()) + return false; + + if (stream->astParent()->astOperand1() != stream) + return false; + + return !astIsIntegral(stream, false); +} + bool isLikelyStreamRead(bool cpp, const Token *op) { if (!cpp) diff --git a/lib/astutils.h b/lib/astutils.h index 34a3e93ca..38c0e69df 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -137,6 +137,10 @@ bool isVariableChanged(const Token *start, const Token *end, const nonneg int va bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp); +bool isAliased(const Token *startTok, const Token *endTok, unsigned int varid); + +bool isAliased(const Variable *var); + /** Determines the number of arguments - if token is a function call or macro * @param start token which is supposed to be the function/macro name. * \return Number of arguments @@ -157,6 +161,8 @@ const Token *findLambdaStartToken(const Token *last); */ const Token *findLambdaEndToken(const Token *first); +bool isLikelyStream(bool cpp, const Token *stream); + /** * do we see a likely write of rhs through overloaded operator * s >> x; diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 5f70bf8bb..e7da51161 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1257,6 +1257,89 @@ void CheckOther::passedByValueError(const Token *tok, const std::string &parname "as a const reference which is usually faster and recommended in C++.", CWE398, inconclusive); } +static bool isUnusedVariable(const Variable *var) +{ + if (!var) + return false; + if (!var->scope()) + return false; + const Token *start = var->declEndToken(); + if (!start) + return false; + if (Token::Match(start, "; %varid% =", var->declarationId())) + start = start->tokAt(2); + return !Token::findmatch(start->next(), "%varid%", var->scope()->bodyEnd, var->declarationId()); +} + +void CheckOther::checkConstVariable() +{ + if (!mSettings->isEnabled(Settings::STYLE) || mTokenizer->isC()) + return; + + const SymbolDatabase *const symbolDatabase = mTokenizer->getSymbolDatabase(); + + for (const Variable *var : symbolDatabase->variableList()) { + if (!var) + continue; + if (!var->isReference()) + continue; + if (var->isRValueReference()) + continue; + if (var->isConst()) + continue; + if (!var->scope()) + continue; + const Scope *scope = var->scope(); + if (!scope->function) + continue; + Function *function = scope->function; + if (var->isArgument()) { + if (function->isImplicitlyVirtual() || function->templateDef) + continue; + if (isUnusedVariable(var)) + continue; + const Token * memberTok = Token::findmatch(function->constructorMemberInitialization(), "%var% ( %varid% )", scope->bodyStart, var->declarationId()); + if (memberTok && memberTok->variable() && memberTok->variable()->isReference()) + continue; + } + if (var->isGlobal()) + continue; + if (var->isStatic()) + continue; + if (var->isArray()) + continue; + if (var->isEnumType()) + continue; + if (var->isVolatile()) + continue; + if (isAliased(var)) + continue; + if (isVariableChanged(var, mSettings, mTokenizer->isCPP())) + continue; + if (Function::returnsReference(function) && + Token::findmatch(var->nameToken(), "return %varid% ;|[", scope->bodyEnd, var->declarationId())) + continue; + // Skip if address is taken + if (Token::findmatch(var->nameToken(), "& %varid%", scope->bodyEnd, var->declarationId())) + continue; + constVariableError(var); + } +} + +void CheckOther::constVariableError(const Variable *var) +{ + const Token *tok = nullptr; + std::string name = "x"; + std::string id = "Variable"; + if (var) { + tok = var->nameToken(); + name = var->name(); + if (var->isArgument()) + id = "Parameter"; + } + reportError(tok, Severity::style, "const" + id, id + " '" + name + "' can be declared with const", CWE398, false); +} + //--------------------------------------------------------------------------- // Check usage of char variables.. //--------------------------------------------------------------------------- diff --git a/lib/checkother.h b/lib/checkother.h index 699431d2c..aac4ec5c9 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -89,6 +89,7 @@ public: checkOther.checkRedundantCopy(); checkOther.clarifyCalculation(); checkOther.checkPassByReference(); + checkOther.checkConstVariable(); checkOther.checkComparisonFunctionIsAlwaysTrueOrFalse(); checkOther.checkInvalidFree(); checkOther.clarifyStatement(); @@ -119,6 +120,8 @@ public: /** @brief %Check for function parameters that should be passed by reference */ void checkPassByReference(); + void checkConstVariable(); + /** @brief Using char variable as array index / as operand in bit operation */ void checkCharVariable(); @@ -218,6 +221,7 @@ private: void cstyleCastError(const Token *tok); void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive); void passedByValueError(const Token *tok, const std::string &parname, bool inconclusive); + void constVariableError(const Variable *var); void constStatementError(const Token *tok, const std::string &type, bool inconclusive); void signedCharArrayIndexError(const Token *tok); void unknownSignCharArrayIndexError(const Token *tok); @@ -288,6 +292,7 @@ private: c.checkCastIntToCharAndBackError(nullptr, "func_name"); c.cstyleCastError(nullptr); c.passedByValueError(nullptr, "parametername", false); + c.constVariableError(nullptr); c.constStatementError(nullptr, "type", false); c.signedCharArrayIndexError(nullptr); c.unknownSignCharArrayIndexError(nullptr); @@ -390,7 +395,8 @@ private: "- find unused 'goto' labels.\n" "- function declaration and definition argument names different.\n" "- function declaration and definition argument order different.\n" - "- shadow variable.\n"; + "- shadow variable.\n" + "- variable can be declared const.\n"; } }; /// @} diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 21b2eea91..5f24db860 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -1731,7 +1731,11 @@ const Type *Variable::smartPointerType() const return nullptr; } -Function::Function(const Tokenizer *mTokenizer, const Token *tok, const Scope *scope, const Token *tokDef, const Token *tokArgDef) +Function::Function(const Tokenizer *mTokenizer, + const Token *tok, + const Scope *scope, + const Token *tokDef, + const Token *tokArgDef) : tokenDef(tokDef), argDef(tokArgDef), token(nullptr), @@ -1745,6 +1749,7 @@ Function::Function(const Tokenizer *mTokenizer, const Token *tok, const Scope *s access(AccessControl::Public), noexceptArg(nullptr), throwArg(nullptr), + templateDef(nullptr), mFlags(0) { // operator function @@ -1801,8 +1806,10 @@ Function::Function(const Tokenizer *mTokenizer, const Token *tok, const Scope *s } // Function template - else if (tok1->link() && tok1->str() == ">" && Token::simpleMatch(tok1->link()->previous(), "template <")) + else if (tok1->link() && tok1->str() == ">" && Token::simpleMatch(tok1->link()->previous(), "template <")) { + templateDef = tok1->link()->previous(); break; + } } // find the return type diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 0a9478b8e..e5bef8c1c 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -855,6 +855,7 @@ public: AccessControl access; ///< public/protected/private const Token *noexceptArg; ///< noexcept token const Token *throwArg; ///< throw token + const Token *templateDef; ///< points to 'template <' before function static bool argsMatch(const Scope *scope, const Token *first, const Token *second, const std::string &path, nonneg int path_length); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index c942a7327..5fd73318b 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -75,6 +75,7 @@ private: TEST_CASE(incorrectLogicOperator11); TEST_CASE(incorrectLogicOperator12); TEST_CASE(incorrectLogicOperator13); + TEST_CASE(incorrectLogicOperator14); TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError); TEST_CASE(incorrectLogicOp_condSwapping); TEST_CASE(testBug5895); @@ -1336,6 +1337,213 @@ private: ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Logical conjunction always evaluates to false: *(v) == 1 && *(x) == 2.\n", errout.str()); } + void incorrectLogicOperator14() { + check("static const std ::string h;\n" + "class i {\n" + "public:\n" + " struct j {\n" + " std ::string k;\n" + " std ::string l;\n" + " };\n" + " struct a {\n" + " enum { m = 1 };\n" + " };\n" + "} b;\n" + "namespace n {\n" + "class c;\n" + "}\n" + "struct o {\n" + " enum { p, d, q, r };\n" + " enum { e, f };\n" + "\n" + "public:\n" + " class j {\n" + " public:\n" + " class s {\n" + " std ::string a;\n" + " };\n" + " };\n" + "};\n" + "namespace n {\n" + "class b;\n" + "}\n" + "namespace aa {\n" + "class d {\n" + "public:\n" + " char t;\n" + " enum {} u;\n" + "};\n" + "} // namespace aa\n" + "namespace aa {\n" + "struct e {};\n" + "} // namespace aa\n" + "class a;\n" + "class w {\n" + "public:\n" + " enum { x };\n" + " struct {\n" + " } y;\n" + " std ::string z;\n" + "};\n" + "class ab {\n" + " friend class c;\n" + "\n" + "public:\n" + " class ac {\n" + " void e(const ac &v) const;\n" + " };\n" + "};\n" + "class f;\n" + "class ad {\n" + " friend class e;\n" + " enum { e, ae, ag, ah, ai, aj, ak, a, b };\n" + " class c {};\n" + " class d {\n" + " enum am { f, an, ao, ap, aq, ar, b, as, at, c, au };\n" + " enum av { aw, ax, ay, az, e, ba, bb, bc, bd, a };\n" + " struct b {\n" + " am action;\n" + " av c;\n" + " };\n" + " };\n" + " class e {\n" + " public:\n" + " std ::string e;\n" + " class f {\n" + " } f;\n" + " class be {\n" + " public:\n" + " };\n" + " std ::vector bf;\n" + " enum { bg, b } c;\n" + " };\n" + " struct bh {\n" + " std ::map b;\n" + " };\n" + " std ::map bi;\n" + " struct {\n" + " int b;\n" + " char bj;\n" + " } bk;\n" + " class a {\n" + " public:\n" + " std ::set b;\n" + " };\n" + "};\n" + "class bl;\n" + "class al;\n" + "class bm;\n" + "class f;\n" + "class b;\n" + "class bn;\n" + "namespace bo {\n" + "class bp {\n" + "public:\n" + " typedef std ::pair bq;\n" + " typedef std ::list br;\n" + "};\n" + "const bo ::bp *dg(const f *a, const al *b);\n" + "} // namespace bo\n" + "const bn *dh(const f *d, bo ::bp ::br &bs);\n" + "class f {\n" + "public:\n" + " struct bt {};\n" + " std ::vector f;\n" + "};\n" + "class bu;\n" + "class a;\n" + "class c;\n" + "struct bv {};\n" + "class af {\n" + "private:\n" + "public:\n" + " enum { b, d, e, f, c, bw };\n" + " void a(int c);\n" + " af *bx() const;\n" + "};\n" + "namespace by {\n" + "class b;\n" + "}\n" + "class b {\n" + "public:\n" + " bool d, c;\n" + "};\n" + "class bz;\n" + "class f;\n" + "class ca {\n" + " friend class b;\n" + "\n" + "public:\n" + " const bm *cb() const { return cc; }\n" + " f *d(f *e, bool f) const;\n" + " int e() { return ++cd; }\n" + " bl *const c;\n" + " bm *cc;\n" + " std ::map ce;\n" + " int cd;\n" + " bz *a;\n" + "};\n" + "namespace n {\n" + "class c;\n" + "class d;\n" + "} // namespace n\n" + "class cf {\n" + "public:\n" + " explicit cf(const std ::string &aname);\n" + " cf(const std ::string &aname, const ca *cg, const al *ch, bl *ci)\n" + " : cj(cg), ck(ch), cl(ci), cn(aname) {}\n" + "\n" + "protected:\n" + " const ca *const cj;\n" + " const al *const ck;\n" + " bl *const cl;\n" + " const std ::string cn;\n" + "};\n" + "class cm : public cf {\n" + "public:\n" + " void cp();\n" + " std ::string d() const;\n" + "};\n" + "struct co {\n" + " co();\n" + " const bu *a;\n" + " enum f {};\n" + " enum {\n" + " b = (1 << 0),\n" + " c = (1 << 1),\n" + " };\n" + " void d(bool e);\n" + "};\n" + "class bu {\n" + " friend class e;\n" + "\n" + "public:\n" + " struct f {};\n" + " enum { d, cr, cq, ct, cs, e, a, b, c, dd, cu, cv, cw, cx, cy, cz, da };\n" + " const f *db;\n" + " const af *dc;\n" + "} f{};\n" + "class bm {\n" + "public:\n" + " std ::list df;\n" + " std ::vector de;\n" + " mutable std ::set f;\n" + "};\n" + "void cm ::cp() {\n" + " const bm *a = cj->cb();\n" + " for (const bu *b : a->de)\n" + " for (af *c = b->dc->bx();;) {\n" + " af *d = c;\n" + " af *e = c;\n" + " bool f(d);\n" + " bool g(e);\n" + " if (f && g)\n" + " ;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void secondAlwaysTrueFalseWhenFirstTrueError() { check("void f(int x) {\n" " if (x > 5 && x != 1)\n" diff --git a/test/testother.cpp b/test/testother.cpp index c24cf1e85..19da51bb9 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -93,6 +93,8 @@ private: TEST_CASE(passedByValue); TEST_CASE(passedByValue_nonConst); + TEST_CASE(constVariable); + TEST_CASE(switchRedundantAssignmentTest); TEST_CASE(switchRedundantOperationTest); TEST_CASE(switchRedundantBitwiseOperationTest); @@ -1567,7 +1569,7 @@ private: check("void f(std::string str) {\n" " std::string& s2 = str;\n" "}"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Variable 's2' can be declared with const\n", errout.str()); check("void f(std::string str) {\n" " const std::string& s2 = str;\n" @@ -1690,6 +1692,360 @@ private: } } + void constVariable() { + check("int f(std::vector x) {\n" + " int& i = x[0];\n" + " return i;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Variable 'i' can be declared with const\n", errout.str()); + + check("int f(std::vector& x) {\n" + " return x[0];\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'x' can be declared with const\n", errout.str()); + + check("int f(std::vector x) {\n" + " const int& i = x[0];\n" + " return i;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int f(std::vector x) {\n" + " static int& i = x[0];\n" + " return i;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int f(std::vector x) {\n" + " int& i = x[0];\n" + " i++;\n" + " return i;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int& f(std::vector& x) {\n" + " x.push_back(1);\n" + " int& i = x[0];\n" + " return i;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int f(const std::vector& x) {\n" + " return x[0];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int& f(std::vector& x) {\n" + " return x[0];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int f(std::vector& x) {\n" + " x[0]++;\n" + " return x[0];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { int a; };\n" + "A f(std::vector& x) {\n" + " x[0].a = 1;\n" + " return x[0];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { int a(); };\n" + "A f(std::vector& x) {\n" + " x[0].a();\n" + " return x[0];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int g(int& x);\n" + "int f(std::vector& x) {\n" + " g(x[0]);\n" + " return x[0];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("template\n" + "T f(T& x) {\n" + " return x[0];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("template\n" + "T f(T&& x) {\n" + " return x[0];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("template\n" + "T f(T& x) {\n" + " return x[0];\n" + "}\n" + "void h() { std::vector v; h(v); }\n"); + ASSERT_EQUALS("", errout.str()); + + check("int f(int& x) {\n" + " return std::move(x);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(std::ostream& os) {\n" + " os << \"Hello\";\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void g(int*);\n" + "void f(int& x) {\n" + " g(&x);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { A(int*); };\n" + "A f(int& x) {\n" + " return A(&x);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { A(int*); };\n" + "A f(int& x) {\n" + " return A{&x};\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // Perhaps unused variable should be checked as well. + check("void f(int& x, int& y) {\n" + " y++;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A {\n" + " explicit A(int& y) : x(&y) {}\n" + " int * x = nullptr;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A {\n" + " std::vector v;\n" + " void swap(A& a) {\n" + " v.swap(a.v);\n" + " }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A {\n" + " template\n" + " void f();\n" + " template\n" + " void f() const;\n" + "};\n" + "void g(A& a) {\n" + " a.f();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(std::vector& v) {\n" + " for(auto&& x:v)\n" + " x = 1;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(std::vector& v) {\n" + " for(auto x:v)\n" + " x = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared with const\n", errout.str()); + + check("void f(std::vector& v) {\n" + " for(auto& x:v) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared with const\n", errout.str()); + + check("void f(std::vector& v) {\n" + " for(const auto& x:v) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'v' can be declared with const\n", errout.str()); + + check("void f(int& i) {\n" + " int& j = i;\n" + " j++;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(std::vector& v) {\n" + " int& i = v[0];\n" + " i++;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(std::map >& m, unsigned int i) {\n" + " std::map& members = m[i];\n" + " members.clear();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A {\n" + " int& x;\n" + " A(int& y) : x(y)\n" + " {}\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("void e();\n" + "void g(void);\n" + "void h(void);\n" + "void ah(void);\n" + "void ai(void);\n" + "void j(void);\n" + "void e(void);\n" + "void k(void);\n" + "void l(void);\n" + "void m(void);\n" + "void n(void);\n" + "void o(void);\n" + "void q(void);\n" + "void r(void);\n" + "void t(void);\n" + "void u(void);\n" + "void v(void);\n" + "void w(void);\n" + "void z(void);\n" + "void aj(void);\n" + "void am(void);\n" + "void g(void);\n" + "void h(void);\n" + "void ah(void);\n" + "void an(void);\n" + "void e(void);\n" + "void k(void);\n" + "void ao(wchar_t *d);\n" + "void ah(void);\n" + "void e(void);\n" + "void an(void);\n" + "void e(void);\n" + "void k(void);\n" + "void g(void);\n" + "void ah(void);\n" + "void an(void);\n" + "void e(void);\n" + "void e(void);\n" + "void e(void);\n" + "void k(void);\n" + "void g(void);\n" + "void ah(void);\n" + "void an(void);\n" + "void e(void);\n" + "void e(void);\n" + "void k(void);\n" + "void g(void);\n" + "void h(void);\n" + "void ah(void);\n" + "void an(void);\n" + "void e(void);\n" + "void k(void);\n" + "void e(void);\n" + "void g(void);\n" + "void ah(void);\n" + "void k(void);\n" + "void an(void);\n" + "void e(void);\n" + "void e(void);\n" + "void e(void);\n" + "void k(void);\n" + "void g(void);\n" + "void h(void);\n" + "void ah(void);\n" + "void k(void);\n" + "void an(void);\n" + "void k(void);\n" + "void e(void);\n" + "void g(void);\n" + "void ah(void);\n" + "void e(void);\n" + "void k(void);\n" + "void g(void);\n" + "void h(void);\n" + "void ah(void);\n" + "void an(void);\n" + "void an(void);\n" + "void k(void);\n" + "void e(void);\n" + "void e(void);\n" + "void e(void);\n" + "void g(void);\n" + "void k(void);\n" + "void g(void);\n" + "void h(void);\n" + "void ah(void);\n" + "void an(void);\n" + "void k(void);\n" + "void k(void);\n" + "void e(void);\n" + "void g(void);\n" + "void g(void);\n" + "void ah(void);\n" + "void an(void);\n" + "void e(void);\n" + "void k(void);\n" + "void e(void);\n" + "void ap(wchar_t *c, int d);\n" + "void ah(void);\n" + "void an(void);\n" + "void g(void);\n" + "void h(void);\n" + "void ah(void);\n" + "void aq(char *b, size_t d, char *c, int a);\n" + "void ar(char *b, size_t d, char *c, va_list a);\n" + "void k(void);\n" + "void g(void);\n" + "void g(void);\n" + "void h(void);\n" + "void ah(void);\n" + "void an(void);\n" + "void k(void);\n" + "void k(void);\n" + "void e(void);\n" + "void g(void);\n" + "void g(void);\n" + "void as(std::string s);\n" + "void at(std::ifstream &f);\n" + "void au(std::istream &f);\n" + "void av(std::string &aa, std::wstring &ab);\n" + "void aw(bool b, double x, double y);\n" + "void ax(int i);\n" + "void ay(std::string c, std::wstring a);\n" + "void az(const std::locale &ac);\n" + "void an();\n" + "void ba(std::ifstream &f);\n" + "void bb(std::istream &f) {\n" + "f.read(NULL, 0);\n" + "}\n" + "void h(void) {\n" + "struct tm *tm = 0;\n" + "(void)std::asctime(tm);\n" + "(void)std::asctime(0);\n" + "}\n" + "void bc(size_t ae) {\n" + "wchar_t *ad = 0, *af = 0;\n" + "struct tm *ag = 0; \n" + "(void)std::wcsftime(ad, ae, af, ag);\n" + "(void)std::wcsftime(0, ae, 0, 0);\n" + "}\n" + "void k(void) {}\n" + "void bd(void);\n" + "void be(void);\n" + "void bf(int b);\n" + "void e(void);\n" + "void e(void);\n" + "void bg(wchar_t *p);\n" + "void bh(const std::list &ak, const std::list &al);\n" + "void ah();\n" + "void an();\n" + "void h();\n"); + ASSERT_EQUALS("", errout.str()); + } + void switchRedundantAssignmentTest() { check("void foo()\n" "{\n" @@ -3738,7 +4094,7 @@ private: // #5535: Reference named like its type check("void foo() { UMSConfig& UMSConfig = GetUMSConfiguration(); }"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Variable 'UMSConfig' can be declared with const\n", errout.str()); // #3868 - false positive (same expression on both sides of |) check("void f(int x) {\n" @@ -7755,17 +8111,17 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); - check("bool f(int & x, int& y) {\n" + check("bool f(const int & x, const int& y) {\n" " return &x > &y;\n" "}\n"); ASSERT_EQUALS("", errout.str()); check("int& g();\n" "bool f() {\n" - " int& x = g();\n" - " int& y = g();\n" - " int* xp = &x;\n" - " int* yp = &y;\n" + " const int& x = g();\n" + " const int& y = g();\n" + " const int* xp = &x;\n" + " const int* yp = &y;\n" " return xp > yp;\n" "}\n"); ASSERT_EQUALS("", errout.str());