Skip to content

Commit

Permalink
Add new check: qbytearray-conversion-to-c-style
Browse files Browse the repository at this point in the history
Warns about usage of the implicit `QByteArray::operator const char *()`.
  • Loading branch information
Ahmad Samir authored and gruenich committed Feb 12, 2025
1 parent 32fbfdd commit 4f99414
Show file tree
Hide file tree
Showing 14 changed files with 204 additions and 0 deletions.
3 changes: 3 additions & 0 deletions Changelog
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions CheckSources.generated.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions ClazyTests.generated.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions checks.json
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
26 changes: 26 additions & 0 deletions docs/checks/README-qbytearray-conversion-to-c-style.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions readmes.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/Checks.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -128,6 +129,8 @@ void CheckManager::registerChecks()
registerCheck(check<IfndefDefineTypo>("ifndef-define-typo", ManualCheckLevel, RegisteredCheck::Option_None));
registerCheck(check<IsEmptyVSCount>("isempty-vs-count", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts));
registerCheck(check<JniSignatures>("jni-signatures", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts));
registerCheck(check<QBytearrayConversionToCStyle>("qbytearray-conversion-to-c-style", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts));
registerFixIt(1, "fix-qbytearray-conversion-to-c-style", "qbytearray-conversion-to-c-style");
registerCheck(check<QHashWithCharPointerKey>("qhash-with-char-pointer-key", ManualCheckLevel, RegisteredCheck::Option_VisitsDecls));
registerCheck(check<QPropertyTypeMismatch>("qproperty-type-mismatch", ManualCheckLevel, RegisteredCheck::Option_VisitsDecls));
registerCheck(check<QRequiredResultCandidates>("qrequiredresult-candidates", ManualCheckLevel, RegisteredCheck::Option_VisitsDecls));
Expand Down
77 changes: 77 additions & 0 deletions src/checks/manuallevel/qbytearray-conversion-to-c-style.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
Copyright (C) 2025 Ahmad Samir <[email protected]>
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 <clang/AST/AST.h>

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<CXXMemberCallExpr>(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>(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<DeclRefExpr>(stmt)) { // E.g. QByteArray ba = "foo"
std::vector<FixItHint> fixits = {clazy::createReplacement(sr, str + ".constData()")};
emitWarning(sr.getEnd(), msg, fixits);
return;
}

if (auto *funcCastExpr = clazy::getFirstChildOfType<CXXFunctionalCastExpr>(stmt)) {
auto *literal = clazy::getFirstChildOfType<StringLiteral>(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<FixItHint> 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<FixItHint> fixits = {clazy::createReplacement(sr, replacement)};
emitWarning(sr.getBegin(), msg, fixits);
}
}
}
22 changes: 22 additions & 0 deletions src/checks/manuallevel/qbytearray-conversion-to-c-style.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
Copyright (C) 2025 Ahmad Samir <[email protected]>
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
9 changes: 9 additions & 0 deletions tests/qbytearray-conversion-to-c-style/config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"tests" : [
{
"filename" : "main.cpp",
"has_fixits" : true,
"qt_major_versions": [6]
}
]
}
22 changes: 22 additions & 0 deletions tests/qbytearray-conversion-to-c-style/main.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#include <QtCore/QByteArray>
#include <QtCore/QString>
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());
}
6 changes: 6 additions & 0 deletions tests/qbytearray-conversion-to-c-style/main.cpp.expected
Original file line number Diff line number Diff line change
@@ -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]
22 changes: 22 additions & 0 deletions tests/qbytearray-conversion-to-c-style/main.cpp.fixed.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#include <QtCore/QByteArray>
#include <QtCore/QString>
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());
}

0 comments on commit 4f99414

Please sign in to comment.