diff --git a/src/checks/level0/qstringref.cpp b/src/checks/level0/qstringref.cpp index e887c9c..94f0a32 100644 --- a/src/checks/level0/qstringref.cpp +++ b/src/checks/level0/qstringref.cpp @@ -1,189 +1,232 @@ /* This file is part of the clazy static checker. Copyright (C) 2015 Sergio Martins This library is free software; you can redistribute it and/or modify it under the terms of the GNU Library General Public License as published by the Free Software Foundation; either version 2 of the License, or (at your option) any later version. This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Library General Public License for more details. You should have received a copy of the GNU Library General Public License along with this library; see the file COPYING.LIB. If not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ #include "qstringref.h" #include "Utils.h" #include "HierarchyUtils.h" #include "checkmanager.h" #include "StringUtils.h" #include "FixItUtils.h" #include #include #include using namespace clang; using namespace std; enum Fixit { FixitNone = 0, FixitUseQStringRef = 0x1, }; StringRefCandidates::StringRefCandidates(const std::string &name, const clang::CompilerInstance &ci) : CheckBase(name, ci) { } static bool isInterestingFirstMethod(CXXMethodDecl *method) { if (!method || method->getParent()->getNameAsString() != "QString") return false; static const vector list = { "left", "mid", "right" }; return clazy_std::contains(list, method->getNameAsString()); } static bool isInterestingSecondMethod(CXXMethodDecl *method, const clang::LangOptions &lo) { if (!method || method->getParent()->getNameAsString() != "QString") return false; static const vector list = { "compare", "contains", "count", "startsWith", "endsWith", "indexOf", "isEmpty", "isNull", "lastIndexOf", "length", "size", "toDouble", "toFloat", "toInt", "toUInt", "toULong", "toULongLong", "toUShort", "toUcs4", "trimmed" }; if (!clazy_std::contains(list, method->getNameAsString())) return false; return !StringUtils::anyArgIsOfAnySimpleType(method, {"QRegExp", "QRegularExpression"}, lo); } static bool isMethodReceivingQStringRef(CXXMethodDecl *method) { static const vector list = { "append", "compare", "count", "indexOf", "endsWith", "lastIndexOf", "localAwareCompare", "startsWidth", "operator+=" }; if (!method || method->getParent()->getNameAsString() != "QString") return false; return clazy_std::contains(list, method->getNameAsString()); } void StringRefCandidates::VisitStmt(clang::Stmt *stmt) { // Here we look for code like str.firstMethod().secondMethod(), where firstMethod() is for example mid() and secondMethod is for example, toInt() auto call = dyn_cast(stmt); if (!call || processCase1(dyn_cast(call))) return; processCase2(call); } +static bool containsChild(Stmt *s, Stmt *target) +{ + if (!s) + return false; + + if (s == target) + return true; + + if (auto mte = dyn_cast(s)) { + return containsChild(mte->getTemporary(), target); + } else if (auto ice = dyn_cast(s)) { + return containsChild(ice->getSubExpr(), target); + } else if (auto bte = dyn_cast(s)) { + return containsChild(bte->getSubExpr(), target); + } + + return false; +} + +bool StringRefCandidates::isConvertedToSomethingElse(clang::Stmt* s) const +{ + // While passing a QString to the QVariant ctor works fine, passing QStringRef doesn't + // So let's not warn when QStrings are cast to something else. + if (!s) + return false; + + auto constr = HierarchyUtils::getFirstParentOfType(m_parentMap, s); + if (!constr || constr->getNumArgs() == 0) + return false; + + if (containsChild(constr->getArg(0), s)) { + CXXConstructorDecl *ctor = constr->getConstructor(); + CXXRecordDecl *record = ctor ? ctor->getParent() : nullptr; + return record ? record->getQualifiedNameAsString() != "QString" : false; + + } + + return false; +} + // Catches cases like: int i = s.mid(1, 1).toInt() bool StringRefCandidates::processCase1(CXXMemberCallExpr *memberCall) { if (!memberCall) return false; // In the AST secondMethod() is parent of firstMethod() call, and will be visited first (because at runtime firstMethod() is resolved first(). // So check for interesting second method first CXXMethodDecl *method = memberCall->getMethodDecl(); if (!isInterestingSecondMethod(method, lo())) return false; vector callExprs = Utils::callListForChain(memberCall); if (callExprs.size() < 2) return false; // The list now contains {secondMethod(), firstMethod() } auto firstMemberCall = dyn_cast(callExprs.at(1)); if (!firstMemberCall || !isInterestingFirstMethod(firstMemberCall->getMethodDecl())) return false; - const string firstMethodName = firstMemberCall->getMethodDecl()->getNameAsString(); + if (isConvertedToSomethingElse(memberCall)) + return false; + + const string firstMethodName = firstMemberCall->getMethodDecl()->getNameAsString(); std::vector fixits; - if (isFixitEnabled(FixitUseQStringRef)) { + if (isFixitEnabled(FixitUseQStringRef)) fixits = fixit(firstMemberCall); - } + emitWarning(firstMemberCall->getLocEnd(), "Use " + firstMethodName + "Ref() instead", fixits); return true; } // Catches cases like: s.append(s2.mid(1, 1)); bool StringRefCandidates::processCase2(CallExpr *call) { auto memberCall = dyn_cast(call); CXXOperatorCallExpr *operatorCall = dyn_cast(call); CXXMethodDecl *method = nullptr; if (memberCall) { method = memberCall->getMethodDecl(); } else if (operatorCall && operatorCall->getCalleeDecl()) { Decl *decl = operatorCall->getCalleeDecl(); method = dyn_cast(decl); } if (!isMethodReceivingQStringRef(method)) return false; Expr *firstArgument = call->getNumArgs() > 0 ? call->getArg(0) : nullptr; MaterializeTemporaryExpr *temp = firstArgument ? dyn_cast(firstArgument) : nullptr; if (!temp) { Expr *secondArgument = call->getNumArgs() > 1 ? call->getArg(1) : nullptr; temp = secondArgument ? dyn_cast(secondArgument) : nullptr; if (!temp) // For the CXXOperatorCallExpr it's in the second argument return false; } CallExpr *innerCall = HierarchyUtils::getFirstChildOfType2(temp); auto innerMemberCall = innerCall ? dyn_cast(innerCall) : nullptr; if (!innerMemberCall) return false; CXXMethodDecl *innerMethod = innerMemberCall->getMethodDecl(); if (!isInterestingFirstMethod(innerMethod)) return false; std::vector fixits; if (isFixitEnabled(FixitUseQStringRef)) { fixits = fixit(innerMemberCall); } emitWarning(call->getLocStart(), "Use " + innerMethod->getNameAsString() + "Ref() instead", fixits); return true; } std::vector StringRefCandidates::fixit(CXXMemberCallExpr *call) { MemberExpr *memberExpr = HierarchyUtils::getFirstChildOfType(call); if (!memberExpr) { queueManualFixitWarning(call->getLocStart(), FixitUseQStringRef, "Internal error 1"); return {}; } auto insertionLoc = Lexer::getLocForEndOfToken(memberExpr->getLocEnd(), 0, sm(), lo()); // llvm::errs() << insertionLoc.printToString(sm()) << "\n"; if (!insertionLoc.isValid()) { queueManualFixitWarning(call->getLocStart(), FixitUseQStringRef, "Internal error 2"); return {}; } std::vector fixits; fixits.push_back(FixItUtils::createInsertion(insertionLoc, "Ref")); return fixits; } const char *const s_checkName = "qstring-ref"; REGISTER_CHECK_WITH_FLAGS(s_checkName, StringRefCandidates, CheckLevel0) REGISTER_FIXIT(FixitUseQStringRef, "fix-missing-qstringref", s_checkName) diff --git a/src/checks/level0/qstringref.h b/src/checks/level0/qstringref.h index a43b102..a054481 100644 --- a/src/checks/level0/qstringref.h +++ b/src/checks/level0/qstringref.h @@ -1,51 +1,52 @@ /* This file is part of the clazy static checker. Copyright (C) 2015 Sergio Martins This library is free software; you can redistribute it and/or modify it under the terms of the GNU Library General Public License as published by the Free Software Foundation; either version 2 of the License, or (at your option) any later version. This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Library General Public License for more details. You should have received a copy of the GNU Library General Public License along with this library; see the file COPYING.LIB. If not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ #ifndef QSTRING_REF_H #define QSTRING_REF_H #include "checkbase.h" namespace clang { class Stmt; class CallExpr; class CXXMemberCallExpr; } /** * Finds places where the QString::fooRef() should be used instead QString::foo(), to save allocations * * See README-qstringref for more info. */ class StringRefCandidates : public CheckBase { public: StringRefCandidates(const std::string &name, const clang::CompilerInstance &ci); void VisitStmt(clang::Stmt *stmt) override; private: bool processCase1(clang::CXXMemberCallExpr*); bool processCase2(clang::CallExpr *call); + bool isConvertedToSomethingElse(clang::Stmt* s) const; std::vector m_alreadyProcessedChainedCalls; std::vector fixit(clang::CXXMemberCallExpr*); }; #endif diff --git a/tests/qstring-ref/config.json b/tests/qstring-ref/config.json index 1078349..a4ad315 100644 --- a/tests/qstring-ref/config.json +++ b/tests/qstring-ref/config.json @@ -1,11 +1,10 @@ { "tests" : [ { "filename" : "main.cpp" }, { - "filename" : "bug376737.cpp", - "expects_failure" : true + "filename" : "bug376737.cpp" } ] }