diff --git a/Changelog b/Changelog index c3f05cb..eb66d1b 100644 --- a/Changelog +++ b/Changelog @@ -1,126 +1,127 @@ * v0.0.1 (June 10th, 2015) - (...) * v1.0 (September 12th, 2016) - (...) * v1.1 (February 20th, 2017) - macOS and Windows support - New checks: child-event-qobject-cast ctor-missing-parent-argument returning-data-from-temporary qt-macros base-class-event connect-non-signal incorrect-emit tr-non-literal - Fixes against clang 4.0 - Fixes against Qt 5.9 - 60% performance improvement - Fixed many false positives * v1.2 (July 8th, 2017) - clazy-standalone executable. Allows to run clazy against a JSON compilation database instead of as a plugin. clang-tidy doesn't support loading external modules (https://bugs.llvm.org/show_bug.cgi?id=32739) so this is a good workaround. - qt-compat mode. Allows to disable Qt5 specific checks by passing -Xclang -plugin-arg-clazy -Xclang qt4-compat - New checks: install-event-filter qcolor-from-literal strict-iterators connect-not-normalized - returning-data-from-temporary now checks for temporary QByteArrays casting to char* when returned - returning-data-from-temporary now checks for assignment too, not only return statements - unused-non-trivial-variable now warns for unused QList, QVector and many more types - ASTMatchers based checks are now supported - clang 3.7 was dropped due to ASTMatchers source incompatibilities. Use clazy v1.1 for clang >= 3.6 support - clazylib.so no longer gets built by default, only the plugin (ClangLazy.so) gets built. Pass -DCLAZY_BUILD_UTILS_LIB=ON to enable the utils library if you're developing tools using clazy's convenience functions, which you're probably not. - CLAZY_INSTALL_NO_HEADERS option was removed. Either install the utils library and headers or nothing at all. By default nothing is installed, except the plugin and man pages. * v1.3 (November 26th, 2017) - New checks: thread-with-slots connect-3arg-lambda qproperty-without-notify virtual-signal overridden-signal qhash-namespace const-signal-or-slot lambda-unique-connection - missing-qobject-macro is now a level2 check, instead of level1. Because, people can omit Q_OBJECT intentionally. - Added -only-qt option, which will make clazy bailout early on non-Qt files. For this purpose, the definition of a Qt file is whenever -DQT_CORE_LIB is passed, which is usually the case in most build systems. - Added -qt-developer option, when building Qt with clazy it will honour specific guidelines for Qt, which are not many right now but the list will grow. * v1.4 (September 23rd, 2018) - New Checks: connect-by-name skipped-base-method qstring-varargs fully-qualified-moc-types qt-keywords, with fixit included qhash-with-char-pointer-key wrong-qevent-cast static-pmf raw-environment-function empty-qstringliteral - auto-unexpected-qstringbuilder now also warns for lambdas returning QStringBuilder - performance optimizations - Added -header-filter= option to clazy-standalone. Only headers matching the regexp will have warnings, besides the .cpp file from the translation unit, which is never filtered out. - Added -ignore-dirs= option to clazy-standalone, and its CLAZY_IGNORE_DIRS env variable equivalent. - Added CLAZY_HEADER_FILTER env variable which adds above functionality to both clazy and clazy-standalone - unused-non-trivial-variable got unused-non-trivial-variable-no-whitelist option - unused-non-trivial-variable got user-blacklist and user-whitelist support - container-inside-loop is now a manual check instead of level2 - HiddenLevel was renamed to ManualLevel - connect-3arg-lambda now warns when passing a lambda to QTimer::singleShot() or QMenu::addAction() without a context object - old-style-connect warns for QMenu::addAction() and QMessageBox::open() too now * v1.5 (January 31st, 2019) - New Checks: ifndef-define-typo lowercase-qml-type-name qrequiredresult-candidates - New Fixits: range-loop now supports adding missing refs or const-ref range-loop now supports adding qAsConst() function-args-by-ref now adding missing refs or const-ref (experimental) Introduced CLAZY_FIXIT_SUFFIX env variable - Removed support for the obscure -DCLAZY_BUILD_UTILS_LIB to simplify the CMakeLists.txt - Renamed the clazy plugin from ClangLazy.so to ClazyPlugin.so - fully-qualified-moc-types now warns for slot/invokable return values too. They need to be fully qualified for QML. - Fixed a crash (clang assert) in raw-environment-function * v1.6 (October 12, 2019) - New Checks: - heap-allocated-small-trivial-type - signal-with-return-value - qproperty-type-mismatch, contributed by Jean-Michaël Celerier - Removed level3. Moved all level3 checks to manual level. Doesn't make sense to enable all of them. Each one must be carefully considered. - Fixed regressions with LLVM 9.0 - Minimum LLVM was bumped to 5.0 - Fixit infrastructure was overhauled Clazy no longer rewrites files directly, to avoid races when parallel invocations change the same header. Clazy now exports a yaml file with the replacements, to be applied with clang-apply-replacements. The same way other clang tooling does it. The way to enable code rewrite is now: -Xclang -plugin-arg-clazy -Xclang export-fixes for clang or -export-fixes=somefile.yaml for clazy-standalone All other fixit arguments and fixit env variables were removed Thanks to Christian Gagneraud for contributing the fixit yaml exporter! * v1.7 (, 2020) - New Checks: - overloaded signal + - qstring-arg warns when using QLatin1String::arg(int), as it casts to QChar diff --git a/docs/checks/README-qstring-arg.md b/docs/checks/README-qstring-arg.md index 819a97c..6625f86 100644 --- a/docs/checks/README-qstring-arg.md +++ b/docs/checks/README-qstring-arg.md @@ -1,39 +1,42 @@ # qstring-arg -Implements two warnings: +Implements three warnings: 1. Detects when you're using chained `QString::arg()` calls and should instead use the multi-arg overload to save memory allocations QString("%1 %2").arg(a).arg(b); QString("%1 %2").arg(a, b); // one less temporary heap allocation -2. Detects when you're using misleading `QString::arg()` overloads +2. Detects when you're passing an integer to QLatin1String::arg() as that gets implicitly cast to QChar. +It's preferable to state your intention and cast to QChar explicitly. + +3. Detects when you're using misleading `QString::arg()` overloads QString arg(qlonglong a, int fieldwidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(qulonglong a, int fieldwidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(long a, int fieldwidth = 0, int base=10, QChar fillChar = QLatin1Char(' ')) const QString arg(ulong a, int fieldwidth = 0, int base=10, QChar fillChar = QLatin1Char(' ')) const QString arg(int a, int fieldWidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(uint a, int fieldWidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(short a, int fieldWidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(ushort a, int fieldWidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(double a, int fieldWidth = 0, char fmt = 'g', int prec = -1, QChar fillChar = QLatin1Char(' ')) const QString arg(char a, int fieldWidth = 0, QChar fillChar = QLatin1Char(' ')) const QString arg(QChar a, int fieldWidth = 0, QChar fillChar = QLatin1Char(' ')) const QString arg(const QString &a, int fieldWidth = 0, QChar fillChar = QLatin1Char(' ')) const because they are commonly misused, for example: int hours = ...; int minutes = ...; // This won't do what you think it would at first glance. QString s("The time is %1:%2").arg(hours, minutes); To reduce false positives, some cases won't be warned about: str.arg(hours, 2); // User explicitly used a integer literal, it's probably fine str.arg(foo); // We're only after cases where the second argument (or further) is specified, so this is safe str.arg(foo, width); // Second argument is named width, or contains the name "width", it's safe. Same for third argument and "base". Using these misleading overloads is perfectly valid, so only warning (1) is enabled by default. To enable warning (2), `export CLAZY_EXTRA_OPTIONS="qstring-arg-fillChar-overloads"` diff --git a/src/checks/level0/qstring-arg.cpp b/src/checks/level0/qstring-arg.cpp index 49752eb..e000245 100644 --- a/src/checks/level0/qstring-arg.cpp +++ b/src/checks/level0/qstring-arg.cpp @@ -1,195 +1,224 @@ /* 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 "qstring-arg.h" #include "Utils.h" #include "StringUtils.h" #include "HierarchyUtils.h" #include "SourceCompatibilityHelpers.h" #include "clazy_stl.h" +#include "ClazyContext.h" +#include "PreProcessorVisitor.h" #include #include #include #include #include #include #include #include #include #include #include class ClazyContext; using namespace clang; using namespace std; QStringArg::QStringArg(const std::string &name, ClazyContext *context) : CheckBase(name, context, Option_CanIgnoreIncludes) { m_filesToIgnore = { "qstring.h" }; + context->enablePreprocessorVisitor(); } static string variableNameFromArg(Expr *arg) { vector declRefs; clazy::getChilds(arg, declRefs); if (declRefs.size() == 1) { ValueDecl *decl = declRefs.at(0)->getDecl(); return decl ? decl->getNameAsString() : string(); } return {}; } -static CXXMethodDecl* isArgMethod(FunctionDecl *func) +static CXXMethodDecl* isArgMethod(FunctionDecl *func, const char *className) { if (!func) return nullptr; auto method = dyn_cast(func); if (!method || clazy::name(method) != "arg") return nullptr; CXXRecordDecl *record = method->getParent(); - if (!record || clazy::name(record) != "QString") + if (!record || clazy::name(record) != className) return nullptr; return method; } static bool isArgFuncWithOnlyQString(CallExpr *callExpr) { if (!callExpr) return false; - CXXMethodDecl *method = isArgMethod(callExpr->getDirectCallee()); + CXXMethodDecl *method = isArgMethod(callExpr->getDirectCallee(), "QString"); if (!method) return false; ParmVarDecl *secondParam = method->getParamDecl(1); if (clazy::classNameFor(secondParam) == "QString") return true; ParmVarDecl *firstParam = method->getParamDecl(0); if (clazy::classNameFor(firstParam) != "QString") return false; // This is a arg(QString, int, QChar) call, it's good if the second parameter is a default param return isa(callExpr->getArg(1)); } bool QStringArg::checkMultiArgWarningCase(const vector &calls) { const int size = calls.size(); for (int i = 1; i < size; ++i) { auto call = calls.at(i); if (calls.at(i - 1)->getNumArgs() + call->getNumArgs() <= 9) { emitWarning(clazy::getLocEnd(call), "Use multi-arg instead"); return true; } } return false; } void QStringArg::checkForMultiArgOpportunities(CXXMemberCallExpr *memberCall) { if (!isArgFuncWithOnlyQString(memberCall)) return; if (clazy::getLocStart(memberCall).isMacroID()) { auto macroName = Lexer::getImmediateMacroName(clazy::getLocStart(memberCall), sm(), lo()); if (macroName == "QT_REQUIRE_VERSION") // bug #391851 return; } vector callExprs = Utils::callListForChain(memberCall); vector argCalls; for (auto call : callExprs) { if (!clazy::contains(m_alreadyProcessedChainedCalls, call) && isArgFuncWithOnlyQString(call)) { argCalls.push_back(call); m_alreadyProcessedChainedCalls.push_back(call); } else { if (checkMultiArgWarningCase(argCalls)) return; argCalls.clear(); } } checkMultiArgWarningCase(argCalls); } +bool QStringArg::checkQLatin1StringCase(CXXMemberCallExpr *memberCall) +{ + PreProcessorVisitor *preProcessorVisitor = m_context->preprocessorVisitor; + if (!preProcessorVisitor || preProcessorVisitor->qtVersion() < 51400) { + // QLatin1String::arg() was introduced in Qt 5.14 + return false; + } + + if (!isArgMethod(memberCall->getDirectCallee(), "QLatin1String")) + return false; + + if (memberCall->getNumArgs() == 0) + return false; + + Expr *arg = memberCall->getArg(0); + QualType t = arg->getType(); + if (!t->isIntegerType() || t->isCharType()) + return false; + + emitWarning(memberCall, "Argument passed to QLatin1String::arg() will be implicitly cast to QChar"); + return true; +} + void QStringArg::VisitStmt(clang::Stmt *stmt) { auto memberCall = dyn_cast(stmt); if (!memberCall) return; if (shouldIgnoreFile(clazy::getLocStart(stmt))) return; checkForMultiArgOpportunities(memberCall); + if (checkQLatin1StringCase(memberCall)) + return; + if (!isOptionSet("fillChar-overloads")) return; - CXXMethodDecl *method = isArgMethod(memberCall->getDirectCallee()); + CXXMethodDecl *method = isArgMethod(memberCall->getDirectCallee(), "QString"); if (!method) return; if (clazy::simpleArgTypeName(method, method->getNumParams() - 1, lo()) == "QChar") { // The second arg wasn't passed, so this is a safe and unambiguous use, like .arg(1) if (isa(memberCall->getArg(1))) return; ParmVarDecl *p = method->getParamDecl(2); if (p && clazy::name(p) == "base") { // User went through the trouble specifying a base, lets allow it if it's a literal. vector literals; clazy::getChilds(memberCall->getArg(2), literals); if (!literals.empty()) return; string variableName = clazy::toLower(variableNameFromArg(memberCall->getArg(2))); if (clazy::contains(variableName, "base")) return; } p = method->getParamDecl(1); if (p && clazy::name(p) == "fieldWidth") { // He specified a literal, so he knows what he's doing, otherwise he would have put it directly in the string vector literals; clazy::getChilds(memberCall->getArg(1), literals); if (!literals.empty()) return; // the variable is named "width", user knows what he's doing string variableName = clazy::toLower(variableNameFromArg(memberCall->getArg(1))); if (clazy::contains(variableName, "width")) return; } emitWarning(clazy::getLocStart(stmt), "Using QString::arg() with fillChar overload"); } } diff --git a/src/checks/level0/qstring-arg.h b/src/checks/level0/qstring-arg.h index f5bbf17..fa51f95 100644 --- a/src/checks/level0/qstring-arg.h +++ b/src/checks/level0/qstring-arg.h @@ -1,53 +1,54 @@ /* 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_ARG_H #define QSTRING_ARG_H #include "checkbase.h" #include #include class ClazyContext; namespace clang { class Stmt; class CXXMemberCallExpr; class CallExpr; } /** * Finds misuse of QString::arg() */ class QStringArg : public CheckBase { public: explicit QStringArg(const std::string &name, ClazyContext *context); void VisitStmt(clang::Stmt *stmt) override; void checkForMultiArgOpportunities(clang::CXXMemberCallExpr *memberCall); private: + bool checkQLatin1StringCase(clang::CXXMemberCallExpr *); bool checkMultiArgWarningCase(const std::vector &calls); std::vector m_alreadyProcessedChainedCalls; }; #endif diff --git a/tests/qstring-arg/config.json b/tests/qstring-arg/config.json index fce4c9d..36ae46b 100644 --- a/tests/qstring-arg/config.json +++ b/tests/qstring-arg/config.json @@ -1,14 +1,18 @@ { "tests" : [ { "filename" : "main.cpp" }, { "filename" : "fill-char-overloads.cpp", "env" : { "CLAZY_EXTRA_OPTIONS" : "qstring-arg-fillChar-overloads" } + }, + { + "filename" : "qlatin1string.cpp", + "minimum_qt_version" : 51400 } ] } diff --git a/tests/qstring-arg/qlatin1string.cpp b/tests/qstring-arg/qlatin1string.cpp new file mode 100644 index 0000000..e84532b --- /dev/null +++ b/tests/qstring-arg/qlatin1string.cpp @@ -0,0 +1,10 @@ +#include + +void test() +{ + int a; + QLatin1String("%1").arg(1); // Warn + QLatin1String("%1").arg(a); // Warn + QLatin1String("%1").arg(QChar(1)); // OK + QLatin1String("%1").arg('a'); // OK +} diff --git a/tests/qstring-arg/qlatin1string.cpp.expected b/tests/qstring-arg/qlatin1string.cpp.expected new file mode 100644 index 0000000..1abe34c --- /dev/null +++ b/tests/qstring-arg/qlatin1string.cpp.expected @@ -0,0 +1,2 @@ +qstring-arg/qlatin1string.cpp:6:5: warning: Argument passed to QLatin1String::arg() will be implicitly cast to QChar [-Wclazy-qstring-arg] +qstring-arg/qlatin1string.cpp:7:5: warning: Argument passed to QLatin1String::arg() will be implicitly cast to QChar [-Wclazy-qstring-arg]