false positive knownConditionTrueFalse around QString::isNull()

[Thomas Friedrichsmeier]

Cppcheck 2.18.3. Consider the following input file:

#include <QString>

int foo(const QString &in) {
    if (in.isNull()) {
        return 1;
    } else if (in.isEmpty()) {
        return 2;
    }
    return 3;
}

Running cppcheck --library=qt --enable=style test.cpp yields:

Checking test.cpp ...
test.cpp:6:26: style: Condition 'in.isEmpty()' is always false [knownConditionTrueFalse]
    } else if (in.isEmpty()) {
                         ^
test.cpp:4:18: note: Assuming that condition 'in.isNull()' is not redundant
    if (in.isNull()) {
                 ^
test.cpp:6:26: note: Condition 'in.isEmpty()' is always false
    } else if (in.isEmpty()) {

However, this is not correct: QString::isNull() does imply QString::isEmpty(), but not so for the negation. Quoting from https://doc.qt.io/qt-6/qstring.html:

QString("").isNull();             // returns false
QString("").isEmpty();            // returns true

[CHR]

See https://trac.cppcheck.net/ticket/12776

[Thomas Friedrichsmeier]

Ah, I had not seen that ticket. I will point out, however, that we checks in that ticket are redundant, indeed, only the redundant check isNull() rather than isEmpty(). In my testcase, neither check is redundant.

I also quite disagree with the ticket's (preliminary) conclusion to mark QString::isNull() as deprecated, however. Yes the Qt documentation states "historical reasons", and recommends avoid QString::isNull(), but no, it is neither deprecated upstream, nor is it useless. It can be put to very good and valid use for a distinction between a known empty string, and a missing (statistics) or uninitialized (lazy/failed initialization) one.

Qt itself specifically documents returning null strings in some cases (QString::mid() starting past end of string; constructing for a nullptr QChar*; QString::clear(); to list just those in the QString-class, itself).

Similar, perhaps even stronger reasoning applies to the related QByteArray, where the documentation also states "historical reasons" for the distinction between null and empty. Yet, e.g. the QSpan-overload of QIODevice::readLineInto() - added quite recently, in Qt 6.9 - specifically documents returning a null QByteArrayView as a way of signalling an error. I will grant that Qt folks are not quite consistent about this, but they clearly don't treat the two as simply the same, or isNull() as deprecated.

TLDR: I repeat: QString::isNull() implies QString::isEmpty(). QString::isEmpty() does not imply QString::isNull()