diff --git a/Changelog b/Changelog index e594202e..27ab8cd6 100644 --- a/Changelog +++ b/Changelog @@ -3,6 +3,9 @@ * v1.14 (TBD) - Clazy warnings for the following line can be disabled, for example: clazy:exclude-next-line=check1,check2 +- New checks: + - qbytearray-conversion-to-c-style + * v1.13 (December 28, 2024) - New Checks: - used-qunused-variable diff --git a/CheckSources.generated.cmake b/CheckSources.generated.cmake index 7bf93420..31b3c7ba 100644 --- a/CheckSources.generated.cmake +++ b/CheckSources.generated.cmake @@ -8,6 +8,7 @@ set(CLAZY_CHECKS_SRCS ${CLAZY_CHECKS_SRCS} ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/ifndef-define-typo.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/isempty-vs-count.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/jnisignatures.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/qbytearray-conversion-to-c-style.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/qhash-with-char-pointer-key.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/qproperty-type-mismatch.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/qrequiredresult-candidates.cpp diff --git a/ClazyTests.generated.cmake b/ClazyTests.generated.cmake index 9babc7c5..42f33a85 100644 --- a/ClazyTests.generated.cmake +++ b/ClazyTests.generated.cmake @@ -23,6 +23,7 @@ add_clazy_test(heap-allocated-small-trivial-type) add_clazy_test(ifndef-define-typo) add_clazy_test(isempty-vs-count) add_clazy_test(jni-signatures) +add_clazy_test(qbytearray-conversion-to-c-style) add_clazy_test(qhash-with-char-pointer-key) add_clazy_test(qproperty-type-mismatch) add_clazy_test(qrequiredresult-candidates) diff --git a/README.md b/README.md index e918fca6..8cb6a0b8 100644 --- a/README.md +++ b/README.md @@ -218,6 +218,7 @@ clazy runs all checks from level1 by default. - [ifndef-define-typo](docs/checks/README-ifndef-define-typo.md) - [isempty-vs-count](docs/checks/README-isempty-vs-count.md) - [jni-signatures](docs/checks/README-jni-signatures.md) + - [qbytearray-conversion-to-c-style](docs/checks/README-qbytearray-conversion-to-c-style.md) (fix-qbytearray-conversion-to-c-style) - [qhash-with-char-pointer-key](docs/checks/README-qhash-with-char-pointer-key.md) - [qproperty-type-mismatch](docs/checks/README-qproperty-type-mismatch.md) - [qrequiredresult-candidates](docs/checks/README-qrequiredresult-candidates.md) diff --git a/checks.json b/checks.json index 39587249..05a6c0a1 100644 --- a/checks.json +++ b/checks.json @@ -1,6 +1,16 @@ { "available_categories" : ["readability", "qt6", "containers", "qstring", "cpp", "bug", "performance", "deprecation", "qml"], "checks" : [ + { + "name" : "qbytearray-conversion-to-c-style", + "level" : -1, + "visits_stmts" : true, + "fixits" : [ + { + "name" : "qbytearray-conversion-to-c-style" + } + ] + }, { "name" : "qt-keywords", "level" : -1, diff --git a/docs/checks/README-qbytearray-conversion-to-c-style.md b/docs/checks/README-qbytearray-conversion-to-c-style.md new file mode 100644 index 00000000..a3491a70 --- /dev/null +++ b/docs/checks/README-qbytearray-conversion-to-c-style.md @@ -0,0 +1,26 @@ +# qbytearray-conversion-to-c-style + +Warns about usage of QByteArray to `const char *` implicit conversion operator. This +sort of conversion is deemed dangerous given how `const char *` is a very common type. +This can be fixed by making the conversion explicit, e.g. by using `QByteArray::constData()`. + +Note that this conversion operator can be disabled by defining the `QBYTEARRAY_NO_CAST_TO_ASCII` +macro. + +#### Caveats +For QByteArrayLiteral this check only works for Qt6 where QByteArrayLiteral is a macro, but not +with Qt5 where QByteArrayLiteral is a lambda. + +#### Example + + void func(const char *); + QByteArray ba = "some text"; + func(ba); // Bad, implicit conversion to const char * + func(ba.constData()); // Good, explicit conversion + + func(QByteArrayLiteral("some literal")); // Bad, implicit conversion plus QByteArray allocation + func("some literal")); // Good, use the string literal directly + +#### Fixits + +This check supports a fixit to rewrite your code. See the README.md on how to enable it. diff --git a/readmes.cmake b/readmes.cmake index 9eb6b672..ad36cbcc 100644 --- a/readmes.cmake +++ b/readmes.cmake @@ -6,6 +6,7 @@ SET(README_manuallevel_FILES ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-ifndef-define-typo.md ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-isempty-vs-count.md ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-jni-signatures.md + ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-qbytearray-conversion-to-c-style.md ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-qhash-with-char-pointer-key.md ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-qproperty-type-mismatch.md ${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-qrequiredresult-candidates.md diff --git a/src/Checks.h b/src/Checks.h index e6a3693d..4b0efdb3 100644 --- a/src/Checks.h +++ b/src/Checks.h @@ -85,6 +85,7 @@ #include "checks/manuallevel/ifndef-define-typo.h" #include "checks/manuallevel/isempty-vs-count.h" #include "checks/manuallevel/jnisignatures.h" +#include "checks/manuallevel/qbytearray-conversion-to-c-style.h" #include "checks/manuallevel/qhash-with-char-pointer-key.h" #include "checks/manuallevel/qproperty-type-mismatch.h" #include "checks/manuallevel/qrequiredresult-candidates.h" @@ -128,6 +129,8 @@ void CheckManager::registerChecks() registerCheck(check("ifndef-define-typo", ManualCheckLevel, RegisteredCheck::Option_None)); registerCheck(check("isempty-vs-count", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts)); registerCheck(check("jni-signatures", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts)); + registerCheck(check("qbytearray-conversion-to-c-style", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts)); + registerFixIt(1, "fix-qbytearray-conversion-to-c-style", "qbytearray-conversion-to-c-style"); registerCheck(check("qhash-with-char-pointer-key", ManualCheckLevel, RegisteredCheck::Option_VisitsDecls)); registerCheck(check("qproperty-type-mismatch", ManualCheckLevel, RegisteredCheck::Option_VisitsDecls)); registerCheck(check("qrequiredresult-candidates", ManualCheckLevel, RegisteredCheck::Option_VisitsDecls)); diff --git a/src/checks/manuallevel/qbytearray-conversion-to-c-style.cpp b/src/checks/manuallevel/qbytearray-conversion-to-c-style.cpp new file mode 100644 index 00000000..482fbcdb --- /dev/null +++ b/src/checks/manuallevel/qbytearray-conversion-to-c-style.cpp @@ -0,0 +1,77 @@ +/* + Copyright (C) 2025 Ahmad Samir + + SPDX-License-Identifier: LGPL-2.0-or-later +*/ + +#include "qbytearray-conversion-to-c-style.h" +#include "ClazyContext.h" +#include "FixItUtils.h" +#include "HierarchyUtils.h" +#include "QtUtils.h" +#include "TypeUtils.h" +#include "Utils.h" + +#include + +using namespace clang; + +QBytearrayConversionToCStyle::QBytearrayConversionToCStyle(const std::string &name, ClazyContext *context) + : CheckBase(name, context, Option_CanIgnoreIncludes) +{ +} + +void QBytearrayConversionToCStyle::VisitStmt(clang::Stmt *stmt) +{ + if (!stmt) + return; + + auto memberExpr = dyn_cast(stmt); + if (!memberExpr) { + return; + } + + auto methodDecl = memberExpr->getMethodDecl(); + if (!methodDecl) + return; + CXXRecordDecl *className = methodDecl->getParent(); + if (!className || className->getName() != "QByteArray") { + return; + } + + constexpr const char byteArrayOp[] = "operator const char *"; + auto *callee = dyn_cast(memberExpr->getCallee()); + if (!callee || callee->getMemberNameInfo().getName().getAsString() != byteArrayOp) { + return; + } + + static const std::string msg = "Don't rely on the QByteArray implicit conversion to 'const char *'."; + + SourceRange sr = callee->getSourceRange(); + CharSourceRange cr = Lexer::getAsCharRange(sr, sm(), lo()); + std::string str = Lexer::getSourceText(cr, sm(), lo()).str(); + if (clazy::getFirstChildOfType(stmt)) { // E.g. QByteArray ba = "foo" + std::vector fixits = {clazy::createReplacement(sr, str + ".constData()")}; + emitWarning(sr.getEnd(), msg, fixits); + return; + } + + if (auto *funcCastExpr = clazy::getFirstChildOfType(stmt)) { + auto *literal = clazy::getFirstChildOfType(funcCastExpr); + if (str.find("QByteArrayLiteral") != std::string_view::npos) { // QByteArrayLiteral("foo"). + // This only works with Qt6 where QByteArrayLiteral is a macro, but not with + // Qt5 where QByteArrayLiteral is a lambda. + std::string replacement = '"' + literal->getString().str() + '"'; // "foo" + // The replacement has to be where the macro is expanded `QByteArrayLiteral("foo")` + // not where it's defined. + CharSourceRange expRange = sm().getExpansionRange(funcCastExpr->getBeginLoc()); + std::vector fixits = {clazy::createReplacement(expRange.getAsRange(), replacement)}; + emitWarning(expRange.getBegin(), msg, fixits); + } else if (str.find("QByteArray") != std::string_view::npos) { // QByteArray("foo") + CharSourceRange cr = Lexer::getAsCharRange(literal->getSourceRange(), sm(), lo()); + std::string replacement = Lexer::getSourceText(cr, sm(), lo()).str(); // "foo" + std::vector fixits = {clazy::createReplacement(sr, replacement)}; + emitWarning(sr.getBegin(), msg, fixits); + } + } +} diff --git a/src/checks/manuallevel/qbytearray-conversion-to-c-style.h b/src/checks/manuallevel/qbytearray-conversion-to-c-style.h new file mode 100644 index 00000000..8cd4bd41 --- /dev/null +++ b/src/checks/manuallevel/qbytearray-conversion-to-c-style.h @@ -0,0 +1,22 @@ +/* + Copyright (C) 2025 Ahmad Samir + + SPDX-License-Identifier: LGPL-2.0-or-later +*/ + +#ifndef CLAZY_QBYTEARRAY_CONVERSION_TO_C_STYLE +#define CLAZY_QBYTEARRAY_CONVERSION_TO_C_STYLE + +#include "checkbase.h" + +/** + * See README-qbytearray-conversion-to-c-style.md for more info. + */ +class QBytearrayConversionToCStyle : public CheckBase +{ +public: + explicit QBytearrayConversionToCStyle(const std::string &name, ClazyContext *context); + void VisitStmt(clang::Stmt *) override; +}; + +#endif // CLAZY_QBYTEARRAY_CONVERSION_TO_C_STYLE diff --git a/tests/qbytearray-conversion-to-c-style/config.json b/tests/qbytearray-conversion-to-c-style/config.json new file mode 100644 index 00000000..0593bc30 --- /dev/null +++ b/tests/qbytearray-conversion-to-c-style/config.json @@ -0,0 +1,9 @@ +{ + "tests" : [ + { + "filename" : "main.cpp", + "has_fixits" : true, + "qt_major_versions": [6] + } + ] +} diff --git a/tests/qbytearray-conversion-to-c-style/main.cpp b/tests/qbytearray-conversion-to-c-style/main.cpp new file mode 100644 index 00000000..45aa6d8a --- /dev/null +++ b/tests/qbytearray-conversion-to-c-style/main.cpp @@ -0,0 +1,22 @@ +#include +#include +static void func(const char *str) { } + +static void otherFunc(const char *str) { } + +#define TEXT "macrotext" + +void test() +{ + QByteArray ba = "this is a qbytearray"; + + const char *c = ba; + func(ba); + + otherFunc(QByteArray("foo")); + otherFunc(QByteArrayLiteral("ba-literal")); + otherFunc(QByteArrayLiteral(TEXT " ba-literal")); + + QString utf16 = QString::fromLatin1("string"); + func(std::exchange(utf16, {}).toLocal8Bit()); +} diff --git a/tests/qbytearray-conversion-to-c-style/main.cpp.expected b/tests/qbytearray-conversion-to-c-style/main.cpp.expected new file mode 100644 index 00000000..17cfb2dd --- /dev/null +++ b/tests/qbytearray-conversion-to-c-style/main.cpp.expected @@ -0,0 +1,6 @@ +qbytearray-conversion-to-c-style/main.cpp:13:21: warning: Don't rely on the QByteArray implicit conversion to 'const char *'. [-Wclazy-qbytearray-conversion-to-c-style] +qbytearray-conversion-to-c-style/main.cpp:14:10: warning: Don't rely on the QByteArray implicit conversion to 'const char *'. [-Wclazy-qbytearray-conversion-to-c-style] +qbytearray-conversion-to-c-style/main.cpp:16:15: warning: Don't rely on the QByteArray implicit conversion to 'const char *'. [-Wclazy-qbytearray-conversion-to-c-style] +qbytearray-conversion-to-c-style/main.cpp:17:15: warning: Don't rely on the QByteArray implicit conversion to 'const char *'. [-Wclazy-qbytearray-conversion-to-c-style] +qbytearray-conversion-to-c-style/main.cpp:18:15: warning: Don't rely on the QByteArray implicit conversion to 'const char *'. [-Wclazy-qbytearray-conversion-to-c-style] +qbytearray-conversion-to-c-style/main.cpp:21:47: warning: Don't rely on the QByteArray implicit conversion to 'const char *'. [-Wclazy-qbytearray-conversion-to-c-style] diff --git a/tests/qbytearray-conversion-to-c-style/main.cpp.fixed.expected b/tests/qbytearray-conversion-to-c-style/main.cpp.fixed.expected new file mode 100644 index 00000000..81a24a86 --- /dev/null +++ b/tests/qbytearray-conversion-to-c-style/main.cpp.fixed.expected @@ -0,0 +1,22 @@ +#include +#include +static void func(const char *str) { } + +static void otherFunc(const char *str) { } + +#define TEXT "macrotext" + +void test() +{ + QByteArray ba = "this is a qbytearray"; + + const char *c = ba.constData(); + func(ba.constData()); + + otherFunc("foo"); + otherFunc("ba-literal"); + otherFunc("macrotext ba-literal"); + + QString utf16 = QString::fromLatin1("string"); + func(std::exchange(utf16, {}).toLocal8Bit().constData()); +}