From 5fc1e9fa0c1925654412af5bf46ff95da99bc190 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 20 Feb 2025 08:02:47 +0100 Subject: [PATCH] tst_QByteArray: check replace() with empty and null needles and haystacks Some time ago, we spent a lot of time fine-tuning the behavior of indexOf() and split() w.r.t. matching of empty needles, or in empty haystacks. Make sure that (QByteArray) replace() is behaving consistently. It isn't. Filed QTBUG-134079 to track the issue. Pick-to: 6.9 6.8 6.5 Task-number: QTBUG-134079 Change-Id: I16af5d2bb5b309a56e81854be434fa9430ae284f Reviewed-by: Thiago Macieira --- .../text/qbytearray/tst_qbytearray.cpp | 103 ++++++++++++++++++ .../tst_qstringapisymmetry.cpp | 100 +++++++++++++++++ 2 files changed, 203 insertions(+) diff --git a/tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp b/tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp index 979b79c6709..6bcf7338694 100644 --- a/tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp +++ b/tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp @@ -77,6 +77,8 @@ private slots: void replace_data(); void replace(); void replaceWithSpecifiedLength(); + void replaceWithEmptyNeedleInsertsBeforeEachChar_data(); + void replaceWithEmptyNeedleInsertsBeforeEachChar(); void number(); void number_double_data(); @@ -1542,6 +1544,107 @@ void tst_QByteArray::replaceWithSpecifiedLength() QCOMPARE(ba,expected); } +void tst_QByteArray::replaceWithEmptyNeedleInsertsBeforeEachChar_data() +{ + QTest::addColumn("haystack"); + QTest::addColumn("needle"); + QTest::addColumn("replacement"); + QTest::addColumn("result"); + + const QByteArray null; + const QByteArray empty = ""; + const QByteArray a = "a"; + const QByteArray aa = "aa"; + const QByteArray b = "b"; + const QByteArray bab = "bab"; + + auto row = [](const QByteArray &haystack, const QByteArray &needle, + const QByteArray &replacement, const QByteArray &result) + { + auto protect = [](const QByteArray &ba) { return ba.isNull() ? "" : ba.data(); }; + QTest::addRow("/%s/%s/%s/", protect(haystack), protect(needle), protect(replacement)) + << haystack << needle << replacement << result; + }; + row(null, null, a, a); + row(null, empty, a, a); + row(null, a, a, null); + row(empty, null, a, a); + row(empty, empty, a, a); + row(empty, a, a, empty); + row(a, null, b, bab); + row(a, empty, b, bab); + row(a, a, b, b); +} + +void tst_QByteArray::replaceWithEmptyNeedleInsertsBeforeEachChar() +{ + QFETCH(const QByteArray, haystack); + QFETCH(const QByteArray, needle); + QFETCH(const QByteArray, replacement); + QFETCH(const QByteArray, result); + + const auto check = [](auto haystack, auto needle, auto replacement, auto result) { + constexpr bool isByteArray = std::is_same_v; + { + // shared + auto copy = haystack; + copy.replace(needle, replacement); + if (isByteArray) { + QEXPECT_FAIL("///a/", "QTBUG-134079", Continue); + QEXPECT_FAIL("///a/", "QTBUG-134079", Continue); + } + QCOMPARE(copy.isNull(), result.isNull()); + if (isByteArray) { + QEXPECT_FAIL("///a/", "QTBUG-134079", Continue); + QEXPECT_FAIL("///a/", "QTBUG-134079", Continue); + } + QCOMPARE(copy, result); + } + { + // unshared + auto copy = haystack; + copy.detach(); + copy.replace(needle, replacement); + // isNull() check pointless, as copy is never isNull() after detach() + QCOMPARE(copy, result); + } + }; + + check(haystack, needle, replacement, result); + if (QTest::currentTestFailed()) + return; + + { + // compared with QString::replace() + const auto h = QString(haystack); + QCOMPARE(h.isNull(), haystack.isNull()); + const auto n = QString(needle); + QCOMPARE(n.isNull(), needle.isNull()); + const auto rep = QString(replacement); + QCOMPARE(rep.isNull(), replacement.isNull()); + const auto res = QString(result); + QCOMPARE(res.isNull(), result.isNull()); + + check(h, n, rep, res); + if (QTest::currentTestFailed()) + return; + } + + { + // compared with QStringTokenizer + QByteArray alt; + for (auto part : qTokenize(QLatin1StringView{haystack}, QLatin1StringView{needle})) { + alt += QByteArrayView{part}; + alt += replacement; + } + if (!alt.isEmpty()) + alt.chop(replacement.size()); // this destroys null'ness + else + QCOMPARE(alt.isNull(), result.isNull()); // so this only makes sense if we didn't chop + QCOMPARE(alt, result); + } +} + void tst_QByteArray::number() { QCOMPARE(QByteArray::number(quint64(0)), QByteArray("0")); diff --git a/tests/auto/corelib/text/qstringapisymmetry/tst_qstringapisymmetry.cpp b/tests/auto/corelib/text/qstringapisymmetry/tst_qstringapisymmetry.cpp index a84b9283381..9d2af3755e8 100644 --- a/tests/auto/corelib/text/qstringapisymmetry/tst_qstringapisymmetry.cpp +++ b/tests/auto/corelib/text/qstringapisymmetry/tst_qstringapisymmetry.cpp @@ -21,6 +21,8 @@ #include +using namespace Qt::StringLiterals; + Q_DECLARE_METATYPE(QLatin1String) namespace { @@ -814,6 +816,14 @@ private Q_SLOTS: void split_QString_char16_t_data() { split_data(false); } void split_QString_char16_t() { split_impl(); } + // tests {QByteArray} × {QByteArray, char}: +#ifdef DOES_NOT_EXIST_YET + void split_QByteArray_QByteArray_data() { split_data(); } + void split_QByteArray_QByteArray() { split_impl(); } +#endif + void split_QByteArray_char_data() { split_data(false); } + void split_QByteArray_char() { split_impl(); } + private: void tok_data(bool rhsHasVariableLength = true); template void tok_impl() const; @@ -899,6 +909,29 @@ private Q_SLOTS: void tok_stdu16string_char16_t_data() { tok_data(false); } void tok_stdu16string_char16_t() { tok_impl(); } +private: + void replace_split_data(bool rhsHasVariableLength = true); + template void replace_split_impl() const; + +private Q_SLOTS: + // tests symmetry between replace() and split()/tokenize() + void replace_split_QString_QString_data() { replace_split_data(); } + void replace_split_QString_QString() { replace_split_impl(); } + void replace_split_QString_QChar_data() { replace_split_data(false); } + void replace_split_QString_QChar() { replace_split_impl(); } + + void replace_split_QByteArray_QByteArray_data() { replace_split_data(); } + void replace_split_QByteArray_QByteArray() { replace_split_impl(); } + void replace_split_QByteArray_char_data() { replace_split_data(false); } + void replace_split_QByteArray_char() { replace_split_impl(); } + +#if 0 // TIL: std::string has no replace(str, str) + void replace_split_stdstring_stdstring_data() { replace_split_data(); } + void replace_split_stdstring_stdstring() { replace_split_impl(); } + void replace_split_stdstring_char_data() { replace_split_data(false); } + void replace_split_stdstring_char() { replace_split_impl(); } +#endif + private: void mid_data(); template void mid_impl(); @@ -1336,6 +1369,16 @@ template constexpr qsizetype size(const T &s) { return qsizetype(s. template <> constexpr qsizetype size(const QChar&) { return 1; } template <> constexpr qsizetype size(const QLatin1Char&) { return 1; } template <> constexpr qsizetype size(const char16_t&) { return 1; } + +template constexpr bool is_null(const T &str) +{ + if constexpr (std::is_pointer_v) + return str == nullptr; + else + return str.isNull(); +} +template <> constexpr bool is_null(const char &str) { return false; } + } // namespace help namespace { @@ -1553,6 +1596,9 @@ template String detached(String s) } return s; } +// std::string does not have a concept of detaching: +template +std::basic_string detached(std::basic_string s) { return s; } template Str make(const QString &s); template <> QString make(const QString &s) { return s; } @@ -2380,10 +2426,12 @@ void tst_QStringApiSymmetry::split_impl() const const auto needle = make(needleU16, needleL1, needleU8); QCOMPARE_EQ(toQStringList(haystack.split(needle)), resultCS); + if constexpr (!std::is_same_v) { QCOMPARE_EQ(toQStringList(haystack.split(needle, Qt::KeepEmptyParts, Qt::CaseSensitive)), resultCS); QCOMPARE_EQ(toQStringList(haystack.split(needle, Qt::KeepEmptyParts, Qt::CaseInsensitive)), resultCIS); QCOMPARE_EQ(toQStringList(haystack.split(needle, Qt::SkipEmptyParts, Qt::CaseSensitive)), skippedResultCS); QCOMPARE_EQ(toQStringList(haystack.split(needle, Qt::SkipEmptyParts, Qt::CaseInsensitive)), skippedResultCIS); + } } void tst_QStringApiSymmetry::tok_data(bool rhsHasVariableLength) @@ -2460,6 +2508,58 @@ void tst_QStringApiSymmetry::tok_impl() const } } +void tst_QStringApiSymmetry::replace_split_data(bool rhsHasVariableLength) +{ + split_data(rhsHasVariableLength); +} + +template +void tst_QStringApiSymmetry::replace_split_impl() const +{ + QFETCH(const QStringView, haystackU16); + QFETCH(const QLatin1String, haystackL1); + QFETCH(const QStringView, needleU16); + QFETCH(const QLatin1String, needleL1); + QFETCH(const QStringList, resultCS); + [[maybe_unused]] // replace() doesn't, yet, have a Qt::CaseSensitivity parameter + QFETCH(const QStringList, resultCIS); + + const auto haystackU8 = haystackU16.toUtf8(); + const auto needleU8 = needleU16.toUtf8(); + + const auto resultCSU16 = resultCS.join(u'a'); + const auto resultCSU8 = resultCSU16.toUtf8(); + const auto resultCSL1 = resultCSU16.toLatin1(); + + const auto haystack = make(haystackU16, haystackL1, haystackU8); + const auto needle = make(needleU16, needleL1, needleU8); + const auto replacement = make(u"a", "a"_L1, "a"_ba); + const auto result = make(resultCSU16, QLatin1StringView{resultCSL1}, resultCSU8); + + QCOMPARE(haystack.isNull(), haystackU16.isNull()); + QCOMPARE(help::is_null(needle), needleU16.isNull()); + QCOMPARE(result.isNull(), resultCSU16.isNull()); + + { + auto copy = haystack; + copy.replace(needle, replacement); + if constexpr (std::is_same_v) { + QEXPECT_FAIL("null ~= null$", "QTBUG-134079", Continue); + QEXPECT_FAIL("null ~= empty$", "QTBUG-134079", Continue); + } + QCOMPARE(copy, result); + } + { + auto copy = detached(haystack); + copy.replace(needle, replacement); + if constexpr (std::is_same_v) { + QEXPECT_FAIL("null ~= null$", "QTBUG-134079", Continue); + QEXPECT_FAIL("null ~= empty$", "QTBUG-134079", Continue); + } + QCOMPARE(copy, result); + } +} + void tst_QStringApiSymmetry::mid_data() { sliced_data();