Utils: fix Text::Range length and remove mid

Those functions are based on the assumption that the passed text starts
at the begin position, which was good enough for search results, but if
used in other parts of the codebase it might give unwanted results.
Calculate the length of the range now as expected and subtract the
beginning lines.
In order to still got the correct results for the text result texts
modify the result range to always start at the first line before
calculating the length of the range.
Also add tests for the modified functionality

Change-Id: I7ccd75b642dda6dd4f738877cbe3543d46c03652
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>

src/libs/utils/textutils.cpp

@@ -58,22 +58,30 @@ Position Position::fromPositionInDocument(const QTextDocument *document, int pos
 
 int Range::length(const QString &text) const
 {
+    if (end.line < begin.line)
+        return -1;
+
     if (begin.line == end.line)
         return end.column - begin.column;
 
-    const int lineCount = end.line - begin.line;
-    int index = text.indexOf(QChar::LineFeed);
+    int index = 0;
     int currentLine = 1;
-    while (index > 0 && currentLine < lineCount) {
-        ++index;
+    while (currentLine < begin.line) {
         index = text.indexOf(QChar::LineFeed, index);
+        if (index < 0)
+            return -1;
+        ++index;
         ++currentLine;
     }
-
-    if (index < 0)
-        return 0;
-
-    return index - begin.column + end.column;
+    const int beginIndex = index + begin.column;
+    while (currentLine < end.line) {
+        index = text.indexOf(QChar::LineFeed, index);
+        if (index < 0)
+            return -1;
+        ++index;
+        ++currentLine;
+    }
+    return index + end.column - beginIndex;
 }
 
 bool Range::operator==(const Range &other) const

src/libs/utils/textutils.h

@@ -37,7 +37,6 @@ public:
 class QTCREATOR_UTILS_EXPORT Range
 {
 public:
-    QString mid(const QString &text) const { return text.mid(begin.column, length(text)); }
     int length(const QString &text) const;
 
     Position begin;
@@ -97,3 +96,4 @@ QTCREATOR_UTILS_EXPORT QString utf16LineTextInUtf8Buffer(const QByteArray &utf8B
 } // Utils
 
 Q_DECLARE_METATYPE(Utils::Text::Position)
+Q_DECLARE_METATYPE(Utils::Text::Range)

src/plugins/coreplugin/find/searchresulttreemodel.cpp

@@ -322,9 +322,13 @@ QVariant SearchResultTreeModel::data(const SearchResultTreeItem *row, int role)
     case ItemDataRoles::ResultBeginColumnNumberRole:
         result = row->item.mainRange().begin.column;
         break;
-    case ItemDataRoles::SearchTermLengthRole:
-        result = row->item.mainRange().length(row->item.lineText());
+    case ItemDataRoles::SearchTermLengthRole:{
+        Text::Range range = row->item.mainRange();
+        range.end.line -= range.begin.line - 1;
+        range.begin.line = 1;
+        result = range.length(row->item.lineText());
         break;
+    }
     case ItemDataRoles::ContainingFunctionNameRole:
         result = row->item.containingFunctionName().value_or(QString{});
         break;

src/plugins/texteditor/basefilefind.cpp

@@ -486,9 +486,13 @@ FilePaths BaseFileFind::replaceAll(const QString &text, const SearchResultItems
             if (item.userData().canConvert<QStringList>() && !item.userData().toStringList().isEmpty()) {
                 replacement = Utils::expandRegExpReplacement(text, item.userData().toStringList());
             } else if (preserveCase) {
-                const QString originalText = (item.mainRange().length(item.lineText()) == 0)
-                                                 ? item.lineText()
-                                                 : item.mainRange().mid(item.lineText());
+                Text::Range range = item.mainRange();
+                range.end.line -= range.begin.line - 1;
+                range.begin.line = 1;
+                QString originalText = item.lineText();
+                const int rangeLength = range.length(item.lineText());
+                if (rangeLength > 0)
+                    originalText = originalText.mid(range.begin.column, rangeLength);
                 replacement = Utils::matchCaseReplacement(originalText, text);
             } else {
                 replacement = text;

tests/auto/utils/CMakeLists.txt

@@ -17,4 +17,5 @@ add_subdirectory(stringutils)
 add_subdirectory(tasktree)
 add_subdirectory(templateengine)
 add_subdirectory(treemodel)
+add_subdirectory(text)
 add_subdirectory(unixdevicefileaccess)

tests/auto/utils/text/CMakeLists.txt

@@ -0,0 +1,4 @@
+add_qtc_test(tst_utils_text
+  DEPENDS Utils
+  SOURCES tst_text.cpp
+)

tests/auto/utils/text/text.qbs

@@ -0,0 +1,7 @@
+import qbs
+
+QtcAutotest {
+    name: "Utils::Text autotest"
+    Depends { name: "Utils" }
+    files: "tst_text.cpp"
+}

tests/auto/utils/text/tst_text.cpp

@@ -0,0 +1,58 @@
+// Copyright (C) 2016 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#include <utils/textutils.h>
+
+#include <QtTest>
+
+using namespace Utils::Text;
+
+class tst_Text : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void testRangeLength_data();
+    void testRangeLength();
+};
+
+void tst_Text::testRangeLength_data()
+{
+    QTest::addColumn<QString>("text");
+    QTest::addColumn<Range>("range");
+    QTest::addColumn<int>("length");
+
+    QTest::newRow("empty range") << QString() << Range{{1, 0}, {1, 0}} << 0;
+
+    QTest::newRow("range on same line") << QString() << Range{{1, 0}, {1, 1}} << 1;
+
+    QTest::newRow("range spanning lines") << QString("foo\nbar") << Range{{1, 0}, {2, 0}} << 4;
+
+    QTest::newRow("range spanning lines and column offset")
+        << QString("foo\nbar") << Range{{1, 1}, {2, 1}} << 4;
+
+    QTest::newRow("range spanning lines and begin column offset")
+        << QString("foo\nbar") << Range{{1, 1}, {2, 0}} << 3;
+
+    QTest::newRow("range spanning lines and end column offset")
+        << QString("foo\nbar") << Range{{1, 0}, {2, 1}} << 5;
+
+    QTest::newRow("hyper range") << QString("foo\nbar\nfoobar") << Range{{2, 1}, {3, 1}} << 4;
+
+    QTest::newRow("flipped range") << QString() << Range{{2, 0}, {1, 0}} << -1;
+
+    QTest::newRow("out of range") << QString() << Range{{1, 0}, {2, 0}} << -1;
+}
+
+void tst_Text::testRangeLength()
+{
+    QFETCH(QString, text);
+    QFETCH(Range, range);
+    QFETCH(int, length);
+
+    QCOMPARE(range.length(text), length);
+}
+
+QTEST_GUILESS_MAIN(tst_Text)
+
+#include "tst_text.moc"