QString(Ref): make toLatin1()/toLocal8Bit() null handling consistent

Systematic testing in tst_QStringApiSymmetry revealed a bug in
QStringRef::toLatin1(): a null input did not result in a null output,
but an empty one. This is fixed, for consistency with
QString::toLatin1(), and QString(Ref)::toUtf8(), which behaved
correctly already.

The same bug was found in QString(Ref)::toLocal8Bit(), which is
particularly hideous, as it's documented to fall back to toLatin1(),
which preserves null inputs. Fixed, too.

[ChangeLog][QtCore][QString] toLocal8Bit() now preserves nullness of
the input QString (outputs null QByteArray).

[ChangeLog][QtCore][QStringRef] toLocal8Bit() and toLatin1() now
preserve nullness of the input QStringRef (output null QByteArrays).

Change-Id: I7026211922c287e03d07e89edbad2987aa646e51
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

5 files changed, 209 insertions, 11 deletions

diff --git a/src/corelib/tools/qstring.cpp b/src/corelib/tools/qstring.cpp
index 606893996c..d76eb11073 100644
--- a/src/corelib/tools/qstring.cpp
+++ b/src/corelib/tools/qstring.cpp
@@ -4635,6 +4635,8 @@ QByteArray QString::toLatin1_helper_inplace(QString &s)
 
 QByteArray QString::toLocal8Bit_helper(const QChar *data, int size)
 {
+    if (!data)
+        return QByteArray();
 #ifndef QT_NO_TEXTCODEC
     QTextCodec *localeCodec = QTextCodec::codecForLocale();
     if (localeCodec)
@@ -10376,6 +10378,8 @@ static inline bool qt_ends_with(const QChar *haystack, int haystackLen,
 */
 QByteArray QStringRef::toLatin1() const
 {
+    if (isNull())
+        return QByteArray();
     return QString::toLatin1_helper(unicode(), length());
 }
 
@@ -10414,9 +10418,11 @@ QByteArray QStringRef::toLatin1() const
 QByteArray QStringRef::toLocal8Bit() const
 {
 #ifndef QT_NO_TEXTCODEC
-    QTextCodec *localeCodec = QTextCodec::codecForLocale();
-    if (localeCodec)
-        return localeCodec->fromUnicode(unicode(), length());
+    if (!isNull()) {
+        QTextCodec *localeCodec = QTextCodec::codecForLocale();
+        if (localeCodec)
+            return localeCodec->fromUnicode(unicode(), length());
+    }
 #endif // QT_NO_TEXTCODEC
     return toLatin1();
 }
diff --git a/src/corelib/tools/qstring.h b/src/corelib/tools/qstring.h
index 08242bff11..00ab1d16b0 100644
--- a/src/corelib/tools/qstring.h
+++ b/src/corelib/tools/qstring.h
@@ -532,9 +532,9 @@ public:
     QByteArray toUtf8() && Q_REQUIRED_RESULT
     { return toUtf8_helper(*this); }
     QByteArray toLocal8Bit() const & Q_REQUIRED_RESULT
-    { return toLocal8Bit_helper(constData(), size()); }
+    { return toLocal8Bit_helper(isNull() ? nullptr : constData(), size()); }
     QByteArray toLocal8Bit() && Q_REQUIRED_RESULT
-    { return toLocal8Bit_helper(constData(), size()); }
+    { return toLocal8Bit_helper(isNull() ? nullptr : constData(), size()); }
 #else
     QByteArray toLatin1() const Q_REQUIRED_RESULT;
     QByteArray toUtf8() const Q_REQUIRED_RESULT;
diff --git a/src/corelib/tools/qstring_compat.cpp b/src/corelib/tools/qstring_compat.cpp
index 300f8467b1..45bb816e4b 100644
--- a/src/corelib/tools/qstring_compat.cpp
+++ b/src/corelib/tools/qstring_compat.cpp
@@ -80,7 +80,7 @@ QByteArray QString::toLatin1() const
 
 QByteArray QString::toLocal8Bit() const
 {
-    return toLocal8Bit_helper(constData(), size());
+    return toLocal8Bit_helper(isNull() ? nullptr : constData(), size());
 }
 
 QByteArray QString::toUtf8() const
diff --git a/tests/auto/corelib/tools/qstring/tst_qstring.cpp b/tests/auto/corelib/tools/qstring/tst_qstring.cpp
index 727eda7456..48e856f406 100644
--- a/tests/auto/corelib/tools/qstring/tst_qstring.cpp
+++ b/tests/auto/corelib/tools/qstring/tst_qstring.cpp
@@ -4273,11 +4273,7 @@ void tst_QString::local8Bit_data()
     QTest::addColumn<QString>("local8Bit");
     QTest::addColumn<QByteArray>("result");
 
-/*
-    QString::local8Bit() called on a null QString returns an _empty_
-    QByteArray.
-*/
-    QTest::newRow("nullString") << QString() << QByteArray("");
+    QTest::newRow("nullString") << QString() << QByteArray();
     QTest::newRow("emptyString") << QString("") << QByteArray("");
     QTest::newRow("string") << QString("test") << QByteArray("test");
 
diff --git a/tests/auto/corelib/tools/qstringapisymmetry/tst_qstringapisymmetry.cpp b/tests/auto/corelib/tools/qstringapisymmetry/tst_qstringapisymmetry.cpp
index 7305a00d94..9d9b47b61e 100644
--- a/tests/auto/corelib/tools/qstringapisymmetry/tst_qstringapisymmetry.cpp
+++ b/tests/auto/corelib/tools/qstringapisymmetry/tst_qstringapisymmetry.cpp
@@ -34,6 +34,7 @@
 #include <QChar>
 #include <QStringRef>
 #include <QLatin1String>
+#include <QVector>
 
 #include <QTest>
 
@@ -71,6 +72,10 @@ class tst_QStringApiSymmetry : public QObject
 {
     Q_OBJECT
 
+    //
+    // Mixed UTF-16, UTF-8, Latin-1 checks:
+    //
+
     void compare_data(bool hasConceptOfNullAndEmpty=true);
     template <typename LHS, typename RHS>
     void compare_impl() const;
@@ -155,6 +160,44 @@ private Q_SLOTS:
     //void compare_const_char_star_const_char_star_data() { compare_data(); }
     //void compare_const_char_star_const_char_star() { compare_impl<const char *, const char *>(); }
 
+    //
+    // UTF-16-only checks:
+    //
+private:
+
+    void toLocal8Bit_data();
+    template <typename String> void toLocal8Bit_impl();
+
+    void toLatin1_data();
+    template <typename String> void toLatin1_impl();
+
+    void toUtf8_data();
+    template <typename String> void toUtf8_impl();
+
+    void toUcs4_data();
+    template <typename String> void toUcs4_impl();
+
+private Q_SLOTS:
+
+    void toLocal8Bit_QString_data() { toLocal8Bit_data(); }
+    void toLocal8Bit_QString() { toLocal8Bit_impl<QString>(); }
+    void toLocal8Bit_QStringRef_data() { toLocal8Bit_data(); }
+    void toLocal8Bit_QStringRef() { toLocal8Bit_impl<QStringRef>(); }
+
+    void toLatin1_QString_data() { toLatin1_data(); }
+    void toLatin1_QString() { toLatin1_impl<QString>(); }
+    void toLatin1_QStringRef_data() { toLatin1_data(); }
+    void toLatin1_QStringRef() { toLatin1_impl<QStringRef>(); }
+
+    void toUtf8_QString_data() { toUtf8_data(); }
+    void toUtf8_QString() { toUtf8_impl<QString>(); }
+    void toUtf8_QStringRef_data() { toUtf8_data(); }
+    void toUtf8_QStringRef() { toUtf8_impl<QStringRef>(); }
+
+    void toUcs4_QString_data() { toUcs4_data(); }
+    void toUcs4_QString() { toUcs4_impl<QString>(); }
+    void toUcs4_QStringRef_data() { toUcs4_data(); }
+    void toUcs4_QStringRef() { toUcs4_impl<QStringRef>(); }
 };
 
 void tst_QStringApiSymmetry::compare_data(bool hasConceptOfNullAndEmpty)
@@ -257,6 +300,159 @@ void tst_QStringApiSymmetry::compare_impl() const
 #undef CHECK
 }
 
+//
+//
+// UTF-16-only checks:
+//
+//
+
+template <class Str> Str  make(const QString &s);
+template <> QStringRef    make(const QString &s)   { return QStringRef(&s); }
+template <> QString       make(const QString &s)   { return s; }
+
+#define REPEAT_16X(X) X X X X  X X X X   X X X X  X X X X
+#define LONG_STRING_256 REPEAT_16X("0123456789abcdef")
+
+void tst_QStringApiSymmetry::toLocal8Bit_data()
+{
+    QTest::addColumn<QString>("unicode");
+    QTest::addColumn<QByteArray>("local");
+
+    auto add = [](const char *local) {
+        const QByteArray ba(local);
+        QString s;
+        for (char c : ba)
+            s += QLatin1Char(c);
+        QTest::addRow("\"%s\" (%d)", ba.left(16).constData(), ba.size()) << s << ba;
+    };
+
+    QTest::addRow("null") << QString() << QByteArray();
+    QTest::addRow("empty") << QString("") << QByteArray("");
+
+    add("Moebius");
+    add(LONG_STRING_256);
+}
+
+template <typename String>
+void tst_QStringApiSymmetry::toLocal8Bit_impl()
+{
+    QFETCH(const QString, unicode);
+    QFETCH(const QByteArray, local);
+
+    const auto str = make<String>(unicode);
+
+    const auto result = str.toLocal8Bit();
+
+    QCOMPARE(result, local);
+    QCOMPARE(unicode.isEmpty(), result.isEmpty());
+    QCOMPARE(unicode.isNull(), result.isNull());
+}
+
+void tst_QStringApiSymmetry::toLatin1_data()
+{
+    QTest::addColumn<QString>("unicode");
+    QTest::addColumn<QByteArray>("latin1");
+
+    auto add = [](const char *l1) {
+        const QByteArray ba(l1);
+        QString s;
+        for (char c : ba)
+            s += QLatin1Char(c);
+        QTest::addRow("\"%s\" (%d)", ba.left(16).constData(), ba.size()) << s << ba;
+    };
+
+    QTest::addRow("null") << QString() << QByteArray();
+    QTest::addRow("empty") << QString("") << QByteArray("");
+
+    add("M\xF6" "bius");
+    add(LONG_STRING_256);
+}
+
+template <typename String>
+void tst_QStringApiSymmetry::toLatin1_impl()
+{
+    QFETCH(const QString, unicode);
+    QFETCH(const QByteArray, latin1);
+
+    const auto str = make<String>(unicode);
+
+    const auto result = str.toLatin1();
+
+    QCOMPARE(result, latin1);
+    QCOMPARE(unicode.isEmpty(), result.isEmpty());
+    QCOMPARE(unicode.isNull(), result.isNull());
+}
+
+void tst_QStringApiSymmetry::toUtf8_data()
+{
+    QTest::addColumn<QString>("unicode");
+    QTest::addColumn<QByteArray>("utf8");
+
+    auto add = [](const char *u8) {
+        QByteArray ba(u8);
+        QString s = ba;
+        QTest::addRow("\"%s\" (%d)", ba.left(16).constData(), ba.size()) << s << ba;
+    };
+
+    QTest::addRow("null") << QString() << QByteArray();
+    QTest::addRow("empty") << QString("") << QByteArray("");
+
+    add("M\xC3\xB6" "bius");
+    add(LONG_STRING_256);
+}
+
+template <typename String>
+void tst_QStringApiSymmetry::toUtf8_impl()
+{
+    QFETCH(const QString, unicode);
+    QFETCH(const QByteArray, utf8);
+
+    const auto str = make<String>(unicode);
+
+    const auto result = str.toUtf8();
+
+    QCOMPARE(result, utf8);
+    QCOMPARE(unicode.isEmpty(), result.isEmpty());
+    QCOMPARE(unicode.isNull(), result.isNull());
+}
+
+void tst_QStringApiSymmetry::toUcs4_data()
+{
+    QTest::addColumn<QString>("unicode");
+    QTest::addColumn<QVector<uint>>("ucs4");
+
+    auto add = [](const char *l1) {
+        const QByteArray ba(l1);
+        QString s;
+        QVector<uint> ucs4;
+        for (char c : ba) {
+            s += QLatin1Char(c);
+            ucs4.append(uint(uchar(c)));
+        }
+        QTest::addRow("\"%s\" (%d)", ba.left(16).constData(), ba.size()) << s << ucs4;
+    };
+
+    QTest::addRow("null") << QString() << QVector<uint>();
+    QTest::addRow("empty") << QString("") << QVector<uint>();
+
+    add("M\xF6" "bius");
+    add(LONG_STRING_256);
+}
+
+template <typename String>
+void tst_QStringApiSymmetry::toUcs4_impl()
+{
+    QFETCH(const QString, unicode);
+    QFETCH(const QVector<uint>, ucs4);
+
+    const auto str = make<String>(unicode);
+
+    const auto result = str.toUcs4();
+
+    QCOMPARE(result, ucs4);
+    QCOMPARE(unicode.isEmpty(), ucs4.isEmpty());
+}
+
 QTEST_APPLESS_MAIN(tst_QStringApiSymmetry)
 
 #include "tst_qstringapisymmetry.moc"