Detect shadowed arguments

This commit is contained in:
Daniel Marjamäki 2019-07-17 17:08:42 +02:00
parent 0be78bbde6
commit d11d6f112e
4 changed files with 34 additions and 13 deletions

View File

@ -2798,26 +2798,43 @@ void CheckOther::checkShadowVariables()
for (const Scope & scope : symbolDatabase->scopeList) { for (const Scope & scope : symbolDatabase->scopeList) {
if (!scope.isExecutable() || scope.type == Scope::eLambda) if (!scope.isExecutable() || scope.type == Scope::eLambda)
continue; continue;
const Scope *functionScope = &scope;
while (functionScope && functionScope->type != Scope::ScopeType::eFunction && functionScope->type != Scope::ScopeType::eLambda)
functionScope = functionScope->nestedIn;
for (const Variable &var : scope.varlist) { for (const Variable &var : scope.varlist) {
if (functionScope && functionScope->type == Scope::ScopeType::eFunction && functionScope->function) {
bool shadowArg = false;
for (const Variable &arg : functionScope->function->argumentList) {
if (arg.nameToken() && var.name() == arg.name()) {
shadowError(var.nameToken(), arg.nameToken(), "argument");
shadowArg = true;
break;
}
}
if (shadowArg)
continue;
}
const Token *shadowed = findShadowed(scope.nestedIn, var.name(), var.nameToken()->linenr()); const Token *shadowed = findShadowed(scope.nestedIn, var.name(), var.nameToken()->linenr());
if (!shadowed) if (!shadowed)
continue; continue;
if (scope.type == Scope::eFunction && scope.className == var.name()) if (scope.type == Scope::eFunction && scope.className == var.name())
continue; continue;
shadowError(var.nameToken(), shadowed, shadowed->varId() != 0); shadowError(var.nameToken(), shadowed, (shadowed->varId() != 0) ? "variable" : "function");
} }
} }
} }
void CheckOther::shadowError(const Token *var, const Token *shadowed, bool shadowVar) void CheckOther::shadowError(const Token *var, const Token *shadowed, std::string type)
{ {
ErrorPath errorPath; ErrorPath errorPath;
errorPath.push_back(ErrorPathItem(shadowed, "Shadowed declaration")); errorPath.push_back(ErrorPathItem(shadowed, "Shadowed declaration"));
errorPath.push_back(ErrorPathItem(var, "Shadow variable")); errorPath.push_back(ErrorPathItem(var, "Shadow variable"));
const std::string &varname = var ? var->str() : (shadowVar ? "var" : "f"); const std::string &varname = var ? var->str() : type;
const char *id = shadowVar ? "shadowVar" : "shadowFunction"; const std::string Type = char(std::toupper(type[0])) + type.substr(1);
std::string message = "$symbol:" + varname + "\nLocal variable \'$symbol\' shadows outer " + (shadowVar ? "variable" : "function"); const std::string id = "shadow" + Type;
reportError(errorPath, Severity::style, id, message, CWE398, false); const std::string message = "$symbol:" + varname + "\nLocal variable \'$symbol\' shadows outer " + type;
reportError(errorPath, Severity::style, id.c_str(), message, CWE398, false);
} }
static bool isVariableExpression(const Token* tok) static bool isVariableExpression(const Token* tok)

View File

@ -259,7 +259,7 @@ private:
void accessMovedError(const Token *tok, const std::string &varname, const ValueFlow::Value *value, bool inconclusive); void accessMovedError(const Token *tok, const std::string &varname, const ValueFlow::Value *value, bool inconclusive);
void funcArgNamesDifferent(const std::string & functionName, nonneg int index, const Token* declaration, const Token* definition); void funcArgNamesDifferent(const std::string & functionName, nonneg int index, const Token* declaration, const Token* definition);
void funcArgOrderDifferent(const std::string & functionName, const Token * declaration, const Token * definition, const std::vector<const Token*> & declarations, const std::vector<const Token*> & definitions); void funcArgOrderDifferent(const std::string & functionName, const Token * declaration, const Token * definition, const std::vector<const Token*> & declarations, const std::vector<const Token*> & definitions);
void shadowError(const Token *var, const Token *shadowed, bool shadowVar); void shadowError(const Token *var, const Token *shadowed, std::string type);
void constArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value); void constArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value);
void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2); void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2);
@ -323,8 +323,9 @@ private:
c.accessMovedError(nullptr, "v", nullptr, false); c.accessMovedError(nullptr, "v", nullptr, false);
c.funcArgNamesDifferent("function", 1, nullptr, nullptr); c.funcArgNamesDifferent("function", 1, nullptr, nullptr);
c.redundantBitwiseOperationInSwitchError(nullptr, "varname"); c.redundantBitwiseOperationInSwitchError(nullptr, "varname");
c.shadowError(nullptr, nullptr, false); c.shadowError(nullptr, nullptr, "variable");
c.shadowError(nullptr, nullptr, true); c.shadowError(nullptr, nullptr, "function");
c.shadowError(nullptr, nullptr, "argument");
c.constArgumentError(nullptr, nullptr, nullptr); c.constArgumentError(nullptr, nullptr, nullptr);
c.comparePointersError(nullptr, nullptr, nullptr); c.comparePointersError(nullptr, nullptr, nullptr);

View File

@ -634,13 +634,13 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
picojson::object obj = res.get<picojson::object>(); picojson::object obj = res.get<picojson::object>();
const std::string filename = obj["file"].get<std::string>(); const std::string fileName = obj["file"].get<std::string>();
const int64_t lineNumber = obj["linenr"].get<int64_t>(); const int64_t lineNumber = obj["linenr"].get<int64_t>();
const int64_t column = obj["col"].get<int64_t>(); const int64_t column = obj["col"].get<int64_t>();
ErrorLogger::ErrorMessage errmsg; ErrorLogger::ErrorMessage errmsg;
errmsg._callStack.emplace_back(ErrorLogger::ErrorMessage::FileLocation(filename, lineNumber)); errmsg._callStack.emplace_back(ErrorLogger::ErrorMessage::FileLocation(fileName, lineNumber));
errmsg._callStack.back().col = column; errmsg._callStack.back().col = column;
errmsg._id = obj["errorId"].get<std::string>(); errmsg._id = obj["errorId"].get<std::string>();
@ -650,7 +650,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
errmsg._severity = Severity::fromString(severity); errmsg._severity = Severity::fromString(severity);
if (errmsg._severity == Severity::SeverityType::none) if (errmsg._severity == Severity::SeverityType::none)
continue; continue;
errmsg.file0 = filename; errmsg.file0 = fileName;
reportErr(errmsg); reportErr(errmsg);
} }

View File

@ -2918,7 +2918,7 @@ private:
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void foo(int x) {\n" check("void foo(int arg) {\n"
" printf(\"%i\", ({int x = do_something(); x == 0;}));\n" " printf(\"%i\", ({int x = do_something(); x == 0;}));\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
@ -7573,6 +7573,9 @@ private:
" auto f = [](){ int x; }" " auto f = [](){ int x; }"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void f(int x) { int x; }");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Local variable 'x' shadows outer argument\n", errout.str());
} }
void constArgument() { void constArgument() {