comphelper: fix natural string compare function + reorganize tests

Natural string compare function doesn't take into account that
the string can start with a number and in this case it treats it
like a conventional string compare. This change takes this case
into account.

This change also refactores the tests for NaturalStringSorter
class. The previous tet used a mock XBreakIterator and XCollator
implementations to test the functionallity. This is not needed as
we can just use a real one instead, which makes the test more
real as it actually uses a real implementation instead of a mock
implementation, which could differ. This change removes the mock
XCollator and XBreakIterator implementations and moves the test
into a new file - NaturalStringSortTest.cxx

The test is also extended with the new use case where the string
starts with a number.

Change-Id: I32ea055f914c2947e4d979093b32f56170a61102
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134540
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>

2 files changed, 95 insertions, 188 deletions

diff --git a/comphelper/qa/string/NaturalStringSortTest.cxx b/comphelper/qa/string/NaturalStringSortTest.cxx
new file mode 100644
index 000000000000..bfdcaff6e13c
--- /dev/null
+++ b/comphelper/qa/string/NaturalStringSortTest.cxx
@@ -0,0 +1,95 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * This file incorporates work covered by the following license notice:
+ *
+ *   Licensed to the Apache Software Foundation (ASF) under one or more
+ *   contributor license agreements. See the NOTICE file distributed
+ *   with this work for additional information regarding copyright
+ *   ownership. The ASF licenses this file to you under the Apache
+ *   License, Version 2.0 (the "License"); you may not use this file
+ *   except in compliance with the License. You may obtain a copy of
+ *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
+ */
+
+#include <comphelper/string.hxx>
+#include <comphelper/processfactory.hxx>
+#include <cppuhelper/implbase.hxx>
+#include <com/sun/star/i18n/CharType.hpp>
+#include <com/sun/star/i18n/XBreakIterator.hpp>
+#include <com/sun/star/i18n/XCollator.hpp>
+
+#include <unotest/bootstrapfixturebase.hxx>
+#include <cppunit/TestAssert.h>
+#include <cppunit/TestFixture.h>
+#include <cppunit/extensions/HelperMacros.h>
+#include <cppunit/plugin/TestPlugIn.h>
+#include <rtl/ustring.hxx>
+
+using namespace css;
+
+namespace
+{
+class TestStringNaturalCompare : public test::BootstrapFixtureBase
+{
+public:
+    void testNatural()
+    {
+        lang::Locale aLocale;
+        aLocale.Language = "en";
+        aLocale.Country = "US";
+
+        comphelper::string::NaturalStringSorter aSorter(comphelper::getProcessComponentContext(),
+                                                        aLocale);
+
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+0), aSorter.compare("ABC", "ABC"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("ABC", "abc"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("abc", "ABC"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("alongstring", "alongerstring"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("alongerstring", "alongstring"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("Heading 9", "Heading 10"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("Heading 10", "Heading 9"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("July, the 4th", "July, the 10th"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("July, the 10th", "July, the 4th"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("abc08", "abc010"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("abc010", "abc08"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+0), aSorter.compare("apple10apple", "apple10apple"));
+
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("KA1", "KA0"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+0), aSorter.compare("KA1", "KA1"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("KA1", "KA2"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("KA50", "KA5"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("KA50", "KA100"));
+
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("1", "0"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+0), aSorter.compare("1", "1"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("1", "2"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("11", "1"));
+
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("50", "100"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("0", "100000"));
+
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("0", "A"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("A", "0"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("A", "99"));
+
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("00ABC2", "00ABC1"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("00ABC1", "00ABC2"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("00ABC11", "00ABC2"));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("00ABC2", "00ABC11"));
+    }
+
+    CPPUNIT_TEST_SUITE(TestStringNaturalCompare);
+    CPPUNIT_TEST(testNatural);
+    CPPUNIT_TEST_SUITE_END();
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestStringNaturalCompare);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/comphelper/qa/string/test_string.cxx b/comphelper/qa/string/test_string.cxx
index 5d3132756ad5..0a9850ed920f 100644
--- a/comphelper/qa/string/test_string.cxx
+++ b/comphelper/qa/string/test_string.cxx
@@ -35,7 +35,6 @@ namespace {
 class TestString: public CppUnit::TestFixture
 {
 public:
-    void testNatural();
     void testStripStart();
     void testStripEnd();
     void testStrip();
@@ -48,7 +47,6 @@ public:
     void testRemoveAny();
 
     CPPUNIT_TEST_SUITE(TestString);
-    CPPUNIT_TEST(testNatural);
     CPPUNIT_TEST(testStripStart);
     CPPUNIT_TEST(testStripEnd);
     CPPUNIT_TEST(testStrip);
@@ -82,192 +80,6 @@ void TestString::testIsdigitAsciiString()
     CPPUNIT_ASSERT_EQUAL(true, comphelper::string::isdigitAsciiString(""));
 }
 
-using namespace ::com::sun::star;
-
-class testCollator : public cppu::WeakImplHelper< i18n::XCollator >
-{
-public:
-    virtual sal_Int32 SAL_CALL compareSubstring(
-        const OUString& str1, sal_Int32 off1, sal_Int32 len1,
-        const OUString& str2, sal_Int32 off2, sal_Int32 len2) override
-    {
-        return str1.copy(off1, len1).compareTo(str2.subView(off2, len2));
-    }
-    virtual sal_Int32 SAL_CALL compareString(
-        const OUString& str1,
-        const OUString& str2) override
-    {
-        return str1.compareTo(str2);
-    }
-    virtual sal_Int32 SAL_CALL loadDefaultCollator(const lang::Locale&, sal_Int32) override {return 0;}
-    virtual sal_Int32 SAL_CALL loadCollatorAlgorithm(const OUString&,
-        const lang::Locale&, sal_Int32) override {return 0;}
-    virtual void SAL_CALL loadCollatorAlgorithmWithEndUserOption(const OUString&,
-        const lang::Locale&, const uno::Sequence< sal_Int32 >&) override {}
-    virtual uno::Sequence< OUString > SAL_CALL listCollatorAlgorithms(const lang::Locale&) override
-    {
-        return uno::Sequence< OUString >();
-    }
-    virtual uno::Sequence< sal_Int32 > SAL_CALL listCollatorOptions(const OUString&) override
-    {
-        return uno::Sequence< sal_Int32 >();
-    }
-};
-
-#define IS_DIGIT(CHAR) (((CHAR) >= 48) && ((CHAR <= 57)))
-
-class testBreakIterator : public cppu::WeakImplHelper< i18n::XBreakIterator >
-{
-public:
-    virtual sal_Int32 SAL_CALL nextCharacters( const OUString&, sal_Int32,
-        const lang::Locale&, sal_Int16, sal_Int32, sal_Int32& ) override {return -1;}
-    virtual sal_Int32 SAL_CALL previousCharacters( const OUString&, sal_Int32,
-        const lang::Locale&, sal_Int16, sal_Int32, sal_Int32& ) override {return -1;}
-
-    virtual i18n::Boundary SAL_CALL previousWord( const OUString&, sal_Int32,
-        const lang::Locale&, sal_Int16) override
-        { return i18n::Boundary(); }
-    virtual i18n::Boundary SAL_CALL nextWord( const OUString&, sal_Int32,
-        const lang::Locale&, sal_Int16) override
-        { return i18n::Boundary(); }
-    virtual i18n::Boundary SAL_CALL getWordBoundary( const OUString&, sal_Int32,
-        const lang::Locale&, sal_Int16, sal_Bool ) override
-        { return i18n::Boundary(); }
-
-    virtual sal_Bool SAL_CALL isBeginWord( const OUString&, sal_Int32,
-        const lang::Locale&, sal_Int16 ) override
-        { return false; }
-    virtual sal_Bool SAL_CALL isEndWord( const OUString&, sal_Int32,
-        const lang::Locale& , sal_Int16 ) override
-        { return false; }
-    virtual sal_Int16 SAL_CALL getWordType( const OUString&, sal_Int32,
-        const lang::Locale& ) override
-        { return 0; }
-
-    virtual sal_Int32 SAL_CALL beginOfSentence( const OUString&, sal_Int32,
-        const lang::Locale& ) override
-        { return 0; }
-    virtual sal_Int32 SAL_CALL endOfSentence( const OUString& rText, sal_Int32,
-        const lang::Locale& ) override
-        { return rText.getLength(); }
-
-    virtual i18n::LineBreakResults SAL_CALL getLineBreak( const OUString&, sal_Int32,
-        const lang::Locale&, sal_Int32,
-        const i18n::LineBreakHyphenationOptions&,
-        const i18n::LineBreakUserOptions&) override
-    {
-        return i18n::LineBreakResults();
-    }
-
-    virtual sal_Int16 SAL_CALL getScriptType( const OUString&, sal_Int32 ) override { return -1; }
-    virtual sal_Int32 SAL_CALL beginOfScript( const OUString&, sal_Int32,
-        sal_Int16 ) override { return -1; }
-    virtual sal_Int32 SAL_CALL endOfScript( const OUString&, sal_Int32,
-        sal_Int16 ) override { return -1; }
-    virtual sal_Int32 SAL_CALL previousScript( const OUString&, sal_Int32,
-        sal_Int16 ) override { return -1; }
-    virtual sal_Int32 SAL_CALL nextScript( const OUString&, sal_Int32,
-        sal_Int16 ) override { return -1; }
-
-    virtual sal_Int32 SAL_CALL beginOfCharBlock( const OUString&, sal_Int32,
-        const lang::Locale&, sal_Int16 ) override { return -1; }
-    virtual sal_Int32 SAL_CALL endOfCharBlock( const OUString& rText, sal_Int32 nStartPos,
-        const lang::Locale&, sal_Int16 CharType ) override
-    {
-        const sal_Unicode *pStr = rText.getStr()+nStartPos;
-        for (sal_Int32 nI = nStartPos; nI < rText.getLength(); ++nI)
-        {
-            if (CharType == i18n::CharType::DECIMAL_DIGIT_NUMBER && !IS_DIGIT(*pStr))
-                return nI;
-            else if (CharType != i18n::CharType::DECIMAL_DIGIT_NUMBER && IS_DIGIT(*pStr))
-                return nI;
-            ++pStr;
-        }
-        return -1;
-    }
-    virtual sal_Int32 SAL_CALL previousCharBlock( const OUString&, sal_Int32,
-        const lang::Locale&, sal_Int16 ) override { return -1; }
-    virtual sal_Int32 SAL_CALL nextCharBlock( const OUString& rText, sal_Int32 nStartPos,
-        const lang::Locale&, sal_Int16 CharType ) override
-    {
-        const sal_Unicode *pStr = rText.getStr()+nStartPos;
-        for (sal_Int32 nI = nStartPos; nI < rText.getLength(); ++nI)
-        {
-            if (CharType == i18n::CharType::DECIMAL_DIGIT_NUMBER && IS_DIGIT(*pStr))
-                return nI;
-            else if (CharType != i18n::CharType::DECIMAL_DIGIT_NUMBER && !IS_DIGIT(*pStr))
-                return nI;
-            ++pStr;
-        }
-        return -1;
-    }
-};
-
-void TestString::testNatural()
-{
-    using namespace comphelper::string;
-
-    uno::Reference< i18n::XCollator > xCollator(new testCollator);
-    uno::Reference< i18n::XBreakIterator > xBI(new testBreakIterator);
-
-// --- Some generic tests to ensure we do not alter original behavior
-// outside what we want
-    CPPUNIT_ASSERT_EQUAL(
-        static_cast<sal_Int32>(0), compareNatural("ABC", "ABC", xCollator, xBI, lang::Locale())
-    );
-    // Case sensitivity
-    CPPUNIT_ASSERT(
-        compareNatural("ABC", "abc", xCollator, xBI, lang::Locale()) < 0
-    );
-    // Reverse
-    CPPUNIT_ASSERT(
-        compareNatural("abc", "ABC", xCollator, xBI, lang::Locale()) > 0
-    );
-    // First shorter
-    CPPUNIT_ASSERT(
-        compareNatural("alongstring", "alongerstring", xCollator, xBI, lang::Locale()) > 0
-    );
-    // Second shorter
-    CPPUNIT_ASSERT(
-        compareNatural("alongerstring", "alongstring", xCollator, xBI, lang::Locale()) < 0
-    );
-// -- Here we go on natural order, each one is followed by classic compare and the reverse comparison
-    // That's why we originally made the patch
-    CPPUNIT_ASSERT(
-        compareNatural("Heading 9", "Heading 10", xCollator, xBI, lang::Locale()) < 0
-    );
-    // Original behavior
-    CPPUNIT_ASSERT(
-        OUString("Heading 9").compareTo(u"Heading 10") > 0
-    );
-    CPPUNIT_ASSERT(
-        compareNatural("Heading 10", "Heading 9", xCollator, xBI, lang::Locale()) > 0
-    );
-    // Harder
-    CPPUNIT_ASSERT(
-        compareNatural("July, the 4th", "July, the 10th", xCollator, xBI, lang::Locale()) < 0
-    );
-    CPPUNIT_ASSERT(
-        OUString("July, the 4th").compareTo(u"July, the 10th") > 0
-    );
-    CPPUNIT_ASSERT(
-        compareNatural("July, the 10th", "July, the 4th", xCollator, xBI, lang::Locale()) > 0
-    );
-    // Hardest
-    CPPUNIT_ASSERT(
-        compareNatural("abc08", "abc010", xCollator, xBI, lang::Locale()) < 0
-    );
-    CPPUNIT_ASSERT(
-        OUString("abc08").compareTo(u"abc010") > 0
-    );
-    CPPUNIT_ASSERT(
-        compareNatural("abc010", "abc08", xCollator, xBI, lang::Locale()) > 0
-    );
-    CPPUNIT_ASSERT_EQUAL(
-        static_cast<sal_Int32>(0), compareNatural("apple10apple", "apple10apple", xCollator, xBI, lang::Locale())
-    );
-}
-
 void TestString::testStripStart()
 {
     OString aIn("abc");